Git development
 help / color / mirror / Atom feed
* Re: [PATCH 6/5] t9001: check send-email behavior with implicit sender
From: Jeff King @ 2012-11-28 20:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, git
In-Reply-To: <7vk3t5v9q1.fsf@alter.siamese.dyndns.org>

On Wed, Nov 28, 2012 at 10:55:02AM -0800, Junio C Hamano wrote:

> > +test_expect_success $PREREQ,AUTOIDENT 'implicit ident is allowed' '
> > +       clean_fake_sendmail &&
> > +       (sane_unset GIT_AUTHOR_NAME &&
> > +	sane_unset GIT_AUTHOR_EMAIL &&
> > +	sane_unset GIT_COMMITTER_NAME &&
> > +	sane_unset GIT_COMMITTER_EMAIL &&
> > +	GIT_SEND_EMAIL_NOTTY=1 git send-email \
> > +		--smtp-server="$(pwd)/fake.sendmail" \
> > +		--to=to@example.com \
> > +		$patches \
> > +		</dev/null 2>errors
> > +       )
> > +'
> > +
> > +test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email' '
> > +       clean_fake_sendmail &&
> > +       (sane_unset GIT_AUTHOR_NAME &&
> > +	sane_unset GIT_AUTHOR_EMAIL &&
> > +	sane_unset GIT_COMMITTER_NAME &&
> > +	sane_unset GIT_COMMITTER_EMAIL &&
> > +	GIT_SEND_EMAIL_NOTTY=1 && export GIT_SEND_EMAIL_NOTTY &&
> > +	test_must_fail git send-email \
> > +		--smtp-server="$(pwd)/fake.sendmail" \
> > +		$patches </dev/null 2>errors &&
> > +	test_i18ngrep "tell me who you are" errors
> > +       )
> > +'
> 
> The difference between these two tests should solely come from
> AUTOIDENT and nothing else, so it would be better to see their
> command line arguments to match; the former is with --to and the
> latter is without in this patch but I do not think you meant them to
> differ that way.

Yeah, that makes sense. The top one originally was testing that we
still prompted in the autoident case, and so had some echos piped into
send-email. I simplified it to use --to, but I agree it is even better
to show that the commands are identical.

Here's a cleaned up version that makes it more obvious the commands are
the same (it also fixes a few minor whitespace problems on the
indentation, which you can see from the quoting above).

-- >8 --
Subject: [PATCH] t9001: check send-email behavior with implicit sender

We allow send-email to use an implicitly-defined identity
for the sender (because there is still a confirmation step),
but we abort when we cannot generate such an identity. Let's
make sure that we test this.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t9001-send-email.sh | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index c5d66cf..97d6f4c 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -201,6 +201,34 @@ test_expect_success $PREREQ 'Prompting works' '
 		grep "^To: to@example.com\$" msgtxt1
 '
 
+test_expect_success $PREREQ,AUTOIDENT 'implicit ident is allowed' '
+	clean_fake_sendmail &&
+	(sane_unset GIT_AUTHOR_NAME &&
+	sane_unset GIT_AUTHOR_EMAIL &&
+	sane_unset GIT_COMMITTER_NAME &&
+	sane_unset GIT_COMMITTER_EMAIL &&
+	GIT_SEND_EMAIL_NOTTY=1 git send-email \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		--to=to@example.com \
+		$patches </dev/null 2>errors
+	)
+'
+
+test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email' '
+	clean_fake_sendmail &&
+	(sane_unset GIT_AUTHOR_NAME &&
+	sane_unset GIT_AUTHOR_EMAIL &&
+	sane_unset GIT_COMMITTER_NAME &&
+	sane_unset GIT_COMMITTER_EMAIL &&
+	GIT_SEND_EMAIL_NOTTY=1 && export GIT_SEND_EMAIL_NOTTY &&
+	test_must_fail git send-email \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		--to=to@example.com \
+		$patches </dev/null 2>errors &&
+	test_i18ngrep "tell me who you are" errors
+	)
+'
+
 test_expect_success $PREREQ 'tocmd works' '
 	clean_fake_sendmail &&
 	cp $patches tocmd.patch &&
-- 
1.8.0.207.gdf2154c

^ permalink raw reply related

* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: Junio C Hamano @ 2012-11-28 20:08 UTC (permalink / raw)
  To: W. Trevor King
  Cc: Git, Jeff King, Phil Hord, Shawn Pearce, Jens Lehmann, Nahor
In-Reply-To: <20121128194216.GA22202@odin.tremily.us>

"W. Trevor King" <wking@tremily.us> writes:

>> As the command takes other options whose names begin with 'r', I
>> thought the longer term plan was to stop letting "--rebase" squat on
>> short and sweet "-r" and leaving it undocumented (even though the
>> short one was added by mistake) was meant to be the first step in
>> that process.
>> 
>> But maybe I am confusing an undocumented single-letter option from
>> some other subcommand.  Anybody remembers?
>
> Perhaps you are remembering:
>
> On Sun, Nov 11, 2012 at 02:33:45AM -0800, Junio C Hamano wrote:
>> Ah, this reminds me of another thing I noticed when I saw that
>> ...

No.  The discussion might or might not be the "-r" option to
"submodule update", but even if it were so, I wasn't refering to
that exchange but something more in the further past.

^ permalink raw reply

* Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format
From: Simon Oosthoek @ 2012-11-28 20:08 UTC (permalink / raw)
  To: Piotr Krukowiecki; +Cc: git
In-Reply-To: <CAA01CspHAHN7se2oJ2WgcmpuRfoa+9Sx9sUvaPEmQ-Y+kDwHhA@mail.gmail.com>

On 28/11/12 19:02, Piotr Krukowiecki wrote:
>     Is your setting?:
>     PROMPT_COMMAND=__git_ps1
> 
> 
> That's right
>  
> 
>     I believe you need to give 2 parameters in order to use it in
>     PROMPT_COMMAND mode.
> 
> 
> Are you sure? git-prompt.sh says:
> 
> #    3a) In ~/.bashrc set PROMPT_COMMAND=__git_ps1
> #        To customize the prompt, provide start/end arguments
> #        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
> 
> I interpret this as: if you don't want to customize the prompt, the
> simple "PROMPT_COMMAND=__git_ps1" is enough.
>  

I'm sure, although you're right this documentation is wrong...
I guess it must have slipped by when the final changes were made to the
code. (The requirement to have 2 arguments for PROMPT_COMMAND mode were
one of the last changes before being accepted into the release process.)

I've been too busy with other stuff to take another look at this in the
meantime.

perhaps the point should read like this:
#    3a) In ~/.bashrc set PROMPT_COMMAND
#        To customize the prompt, provide start/end arguments
#        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'

Which would not be confusing at all, I think...

> 
> 
>     These last 2 lines say: if 2 arguments are given, use pcmode.
>     Otherwise you get command-subtitution mode, which gives weird
>     effects when being called from PROMPT_COMMAND.
> 
> 
> Still, it seems to work with above "patch"..

It only works in your specific case, since you're expecting the branch
info before the rest of your prompt. The output comes from the part of
the code that is meant for substitution mode ;-)

Cheers

Simon

^ permalink raw reply

* Re: [PATCH 6/5] t9001: check send-email behavior with implicit sender
From: Jeff King @ 2012-11-28 20:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, git
In-Reply-To: <20121128200626.GA4292@sigill.intra.peff.net>

On Wed, Nov 28, 2012 at 03:06:26PM -0500, Jeff King wrote:

> Here's a cleaned up version that makes it more obvious the commands are
> the same (it also fixes a few minor whitespace problems on the
> indentation, which you can see from the quoting above).

I wondered how painful it would be to actually run the command and then
conditionally check the results inside the test. I ended up with this:

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index c5d66cf..9c8fac1 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -201,6 +201,30 @@ test_expect_success $PREREQ 'Prompting works' '
 		grep "^To: to@example.com\$" msgtxt1
 '
 
+test_expect_success $PREREQ 'handle implicit ident' '
+	clean_fake_sendmail &&
+	(
+		sane_unset GIT_AUTHOR_NAME &&
+		sane_unset GIT_AUTHOR_EMAIL &&
+		sane_unset GIT_COMMITTER_NAME &&
+		sane_unset GIT_COMMITTER_EMAIL &&
+		GIT_SEND_EMAIL_NOTTY=1 && export GIT_SEND_EMAIL_NOTTY &&
+		{
+			git send-email \
+				--smtp-server="$(pwd)/fake.sendmail" \
+				--to=to@example.com \
+				$patches </dev/null 2>errors;
+			exit_code=$?
+		} &&
+		if test_have_prereq AUTOIDENT; then
+			test $exit_code = 0
+		else
+			test $exit_code != 0 &&
+			test_i18ngrep "tell me who you are" errors
+		fi
+	)
+'
+
 test_expect_success $PREREQ 'tocmd works' '
 	clean_fake_sendmail &&
 	cp $patches tocmd.patch &&

which does work, though it is less nice that the protections and nice
syntax of "test_must_fail" must be abandoned (unless we want to do
something horrible like 'test_must_fail sh -c "exit $exit_code"'.
I could go either way.

-Peff

^ permalink raw reply related

* Re: git config key bug or by design?
From: Jeff King @ 2012-11-28 20:21 UTC (permalink / raw)
  To: Peter van der Does; +Cc: git
In-Reply-To: <20121128071147.188a869e@Indy>

On Wed, Nov 28, 2012 at 07:11:47AM -0500, Peter van der Does wrote:

> I am writing a tool, it needs to store branch names in a separate config
> file.
> 
> It's clear git doesn't respect those values, hence my question. I
> understand how to work around the problem, I would just prefix the key.
> I was just wondering if it was by design, which I guess it is as the
> parsing of the file will die if the key starts with a non-alpha
> character.

In that case, I would definitely say to use some prefix section like
"section.$branchname.key". That is how git stores per-branch information
(e.g., branch.master.merge), and it was always designed to let there be
arbitrary data in the "middle" section, whereas the section and key are
restricted and case-insensitive.

So no, I do not recall "cannot start with number" as a specific design
decision, but it is definitely a design decision that the section name
would not allow arbitrary content.

-Peff

^ permalink raw reply

* Re: [PATCH v6 07/16] remote-hg: add support for hg-git compat mode
From: W. Trevor King @ 2012-11-28 20:23 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Jeff King, Sverre Rabbelier,
	Johannes Schindelin, Ilari Liusvaara, Daniel Barkalow,
	Michael J Gruber
In-Reply-To: <1351995218-19889-8-git-send-email-felipe.contreras@gmail.com>

I'm not sure if this is the most recent patch iteration for this
feature, but I just saw this typo in `pu`.

On Sun, Nov 04, 2012 at 03:13:29AM +0100, Felipe Contreras wrote:
> +# Commits are modified to preserve hg information and allow biridectionality.
                                                               ^^^^^^^^
s/biridectionality/bidirectionality/

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2012, #09; Wed, 28)
From: Jeff King @ 2012-11-28 20:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v38ztv6z0.fsf@alter.siamese.dyndns.org>

On Wed, Nov 28, 2012 at 11:54:27AM -0800, Junio C Hamano wrote:

> * jk/fsck-dot-in-trees (2012-11-28) 1 commit
>  - fsck: warn about '.' and '..' in trees
> 
>  Will merge to 'next'.

Do you have an opinion on warning about '.git', as well? It probably
would make more sense as a patch on top, but I thought I'd ask before
this got merged to next.

> * pf/editor-ignore-sigint (2012-11-11) 5 commits
>  - launch_editor: propagate SIGINT from editor to git
>  - run-command: do not warn about child death by SIGINT
>  - run-command: drop silent_exec_failure arg from wait_or_whine
>  - launch_editor: ignore SIGINT while the editor has control
>  - launch_editor: refactor to use start/finish_command
> 
>  Avoid confusing cases where the user hits Ctrl-C while in the editor
>  session, not realizing git will receive the signal. Since most editors
>  will take over the terminal and will block SIGINT, this is not likely
>  to confuse anyone.
> 
>  Some people raised issues with emacsclient, which are addressed by this
>  re-roll. It should probably also handle SIGQUIT, and there were a
>  handful of other review comments.
> 
>  Expecting a re-roll.

I'm slowly going through my post-travel/vacation/illness backlog. I hope
to re-roll this one today or tomorrow.

> * jn/warn-on-inaccessible-loosen (2012-10-14) 4 commits
>  - config: exit on error accessing any config file
>  - doc: advertise GIT_CONFIG_NOSYSTEM
>  - config: treat user and xdg config permission problems as errors
>  - config, gitignore: failure to access with ENOTDIR is ok
> 
>  An RFC to deal with a situation where .config/git is a file and we
>  notice .config/git/config is not readable due to ENOTDIR, not
>  ENOENT; I think a bit more refactored approach to consistently
>  address permission errors across config, exclude and attrs is
>  desirable.  Don't we also need a check for an opposite situation
>  where we open .config/git/config or .gitattributes for reading but
>  they turn out to be directories?

I am not sure about the refactored approach you mention. We
fundamentally need to treat in-tree attributes and exclude files more
leniently, because we may find arbitrary paths in the tree. Whereas if
something in $GIT_DIR is inaccessible, it's probably a serious problem.
So I think we have to manually use access_or_{warn,die} from each
callsite.

As far as the opposite situation, I do not know if it is worth worrying
about. If $GIT_DIR/config or $HOME/.config/git/config is a directory,
that is an error and we should flag it[1]. In-tree is more hazy, but I
think complaining is still the right thing. You cannot expect to store
arbitrary crap at .gitattributes inside your tree. If you have crap in a
file at such a path, we would read it and complain when its syntax is
not that of a .gitattributes file. We should similarly complain when it
is a directory.

-Peff

[1] We might want to eventually allow "config directories" where we
    would read all files in lexical order or something. So it is
    tempting to think of ignoring such entries as a
    forward-compatibility thing. But ignoring is the wrong thing; you
    probably would not want an old version of git to _ignore_ your
    config; it is better if it says "I am too old to understand your
    config format".

^ permalink raw reply

* Re: [PATCH 6/5] t9001: check send-email behavior with implicit sender
From: Junio C Hamano @ 2012-11-28 20:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Felipe Contreras, git
In-Reply-To: <20121128201713.GA9249@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Wed, Nov 28, 2012 at 03:06:26PM -0500, Jeff King wrote:
>
>> Here's a cleaned up version that makes it more obvious the commands are
>> the same (it also fixes a few minor whitespace problems on the
>> indentation, which you can see from the quoting above).
>
> I wondered how painful it would be to actually run the command and then
> conditionally check the results inside the test. I ended up with this:
> ...
> which does work, though it is less nice that the protections and nice
> syntax of "test_must_fail" must be abandoned (unless we want to do
> something horrible like 'test_must_fail sh -c "exit $exit_code"'.
> I could go either way.

A change along that line did cross my mind, and I agree that it does
clarify that we are testing the same command produces an expected
result, the expectation may be different depending on external
conditions, and we can figure out what the expected values are
before running the test.  I do not think we use such a pattern in
very many places, though.

The differences in "expected results" are generally not limited to
the final outcome in $? (e.g. your "else" side not just checks $?
indicates an error, but makes sure that we got an expected error
message), which means that the if statement at the end has to be
there in some form anyway, which in turn means that a new test
helper function to abstract this pattern out further would not
buy us very much (either it would be too complex and bug prone to
implement, or it would be too simplistic and limited).

I'd prefer to leave these as two separate tests for now like your
previous patch.  People interested in consolidating/simplifying can
first audit the existing tests to find other instances of this
pattern to see if it is worth doing, and then come up with an
abstraction to be shared among them.

Thanks.

^ permalink raw reply

* Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format
From: Junio C Hamano @ 2012-11-28 20:47 UTC (permalink / raw)
  To: Simon Oosthoek; +Cc: Piotr Krukowiecki, git
In-Reply-To: <50B66F41.1030305@xs4all.nl>

Simon Oosthoek <s.oosthoek@xs4all.nl> writes:

> perhaps the point should read like this:
> #    3a) In ~/.bashrc set PROMPT_COMMAND
> #        To customize the prompt, provide start/end arguments
> #        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
>
> Which would not be confusing at all, I think...

It says "to customize", so a user who just wants the default (which
does not exist but the comment does not say so) would be left
without instruction, no?

    In $HOME/.bashrc, PROMPT_COMMAND can be set to
    '__git_ps1 <pre> <post>', where <pre> and <post>
    are strings you would put in $PS1 before and after
    the status string generated by git-prompt machinery.
    e.g.
        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'

or something?

^ permalink raw reply

* Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format
From: Simon Oosthoek @ 2012-11-28 20:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Piotr Krukowiecki, git
In-Reply-To: <7vlidltpyj.fsf@alter.siamese.dyndns.org>

On 28/11/12 21:47, Junio C Hamano wrote:
> Simon Oosthoek <s.oosthoek@xs4all.nl> writes:
> 
>> perhaps the point should read like this:
>> #    3a) In ~/.bashrc set PROMPT_COMMAND
>> #        To customize the prompt, provide start/end arguments
>> #        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
>>
>> Which would not be confusing at all, I think...
> 
> It says "to customize", so a user who just wants the default (which
> does not exist but the comment does not say so) would be left
> without instruction, no?
> 
>     In $HOME/.bashrc, PROMPT_COMMAND can be set to
>     '__git_ps1 <pre> <post>', where <pre> and <post>
>     are strings you would put in $PS1 before and after
>     the status string generated by git-prompt machinery.
>     e.g.
>         PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
> 
> or something?
> 

Looks better than my suggestion :-)

thanks

/Simon

^ permalink raw reply

* Re: git-svn: What is --follow-parent / --no-follow-parent for?
From: Eric Wong @ 2012-11-28 21:20 UTC (permalink / raw)
  To: Sebastian Leske; +Cc: git
In-Reply-To: <20121120073153.GA340@localhost>

Sebastian Leske <Sebastian.Leske@sleske.name> wrote:
> However, this does not make sense to me: This sounds like there is no
> good reason *not* to enable this option.  So why is it there? And in
> what situation might I want to use "--no-follow-parent"?

Speed.  Following long/convoluted histories can take a long time.
Sometimes the user doesn't care about ancient history.

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2012, #09; Wed, 28)
From: Junio C Hamano @ 2012-11-28 21:25 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20121128203826.GA9383@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Wed, Nov 28, 2012 at 11:54:27AM -0800, Junio C Hamano wrote:
>
>> * jk/fsck-dot-in-trees (2012-11-28) 1 commit
>>  - fsck: warn about '.' and '..' in trees
>> 
>>  Will merge to 'next'.
>
> Do you have an opinion on warning about '.git', as well? It probably
> would make more sense as a patch on top, but I thought I'd ask before
> this got merged to next.

Yeah, it would make sense to reject what we would not record
ourselves when the tools are used in a sane manner.

>> * pf/editor-ignore-sigint (2012-11-11) 5 commits
>>  - launch_editor: propagate SIGINT from editor to git
>>  - run-command: do not warn about child death by SIGINT
>>  - run-command: drop silent_exec_failure arg from wait_or_whine
>>  - launch_editor: ignore SIGINT while the editor has control
>>  - launch_editor: refactor to use start/finish_command
>> 
>>  Avoid confusing cases where the user hits Ctrl-C while in the editor
>>  session, not realizing git will receive the signal. Since most editors
>>  will take over the terminal and will block SIGINT, this is not likely
>>  to confuse anyone.
>> 
>>  Some people raised issues with emacsclient, which are addressed by this
>>  re-roll. It should probably also handle SIGQUIT, and there were a
>>  handful of other review comments.
>> 
>>  Expecting a re-roll.
>
> I'm slowly going through my post-travel/vacation/illness backlog. I hope
> to re-roll this one today or tomorrow.

Thanks.

>> * jn/warn-on-inaccessible-loosen (2012-10-14) 4 commits
>>  - config: exit on error accessing any config file
>>  - doc: advertise GIT_CONFIG_NOSYSTEM
>>  - config: treat user and xdg config permission problems as errors
>>  - config, gitignore: failure to access with ENOTDIR is ok
>> 
>>  An RFC to deal with a situation where .config/git is a file and we
>>  notice .config/git/config is not readable due to ENOTDIR, not
>>  ENOENT; I think a bit more refactored approach to consistently
>>  address permission errors across config, exclude and attrs is
>>  desirable.  Don't we also need a check for an opposite situation
>>  where we open .config/git/config or .gitattributes for reading but
>>  they turn out to be directories?
>
> I am not sure about the refactored approach you mention. We
> fundamentally need to treat in-tree attributes and exclude files more
> leniently, because we may find arbitrary paths in the tree.

OK.

> As far as the opposite situation, I do not know if it is worth worrying
> about. If $GIT_DIR/config or $HOME/.config/git/config is a directory,
> that is an error and we should flag it[1]. In-tree is more hazy, but I
> think complaining is still the right thing. You cannot expect to store
> arbitrary crap at .gitattributes inside your tree. If you have crap in a
> file at such a path, we would read it and complain when its syntax is
> not that of a .gitattributes file. We should similarly complain when it
> is a directory.

Yeah, OK.

^ permalink raw reply

* Re: [PATCH] svnrdump_sim: start the script with /usr/bin/env python
From: Felipe Contreras @ 2012-11-28 21:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Christian Couder, Christian Couder, git
In-Reply-To: <7v7gp5v7y1.fsf@alter.siamese.dyndns.org>

On Wed, Nov 28, 2012 at 8:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Wed, Nov 28, 2012 at 5:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>>> You need a fix for that; didn't I already say "you need a bit more
>>> than that"?
>>
>> I disagree. Most of the contrib scripts are expected to be used as
>> they are.
>
> You are only looking at one of the uses for this script, when there
> are two.
>
> You are correct that distros may install with whatever tweaks of
> their own, and to help their tweak process (like the one that
> specifically notices "/usr/bin/env python" as you wrote), changing
> the "#!/usr/bin/python" to match others would be a good change.
>
> But that change alone is not sufficient for this one, which is used
> from t/ script.  You cannot treat this one like import-zips and
> hg-to-git that we do not use in-tree.  Somewhere before t9020 uses
> it, it needs the treatment similar to the rewriting that is done for
> git-p4.py to git-p4.

Unless the tests are moved to contrib, which I think is a good
practice: should anything in contrib break 'make test'? I don't think
so.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* [PATCH] fsck: warn about ".git" in trees
From: Jeff King @ 2012-11-28 21:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Wed, Nov 28, 2012 at 01:25:05PM -0800, Junio C Hamano wrote:

> >> * jk/fsck-dot-in-trees (2012-11-28) 1 commit
> >>  - fsck: warn about '.' and '..' in trees
> >> 
> >>  Will merge to 'next'.
> >
> > Do you have an opinion on warning about '.git', as well? It probably
> > would make more sense as a patch on top, but I thought I'd ask before
> > this got merged to next.
> 
> Yeah, it would make sense to reject what we would not record
> ourselves when the tools are used in a sane manner.

Here's the patch on top of jk/fsck-dot-in-trees.

-- >8 --
Subject: [PATCH] fsck: warn about ".git" in trees

Having a ".git" entry inside a tree can cause confusing
results on checkout. At the top-level, you could not
checkout such a tree, as it would complain about overwriting
the real ".git" directory. In a subdirectory, you might
check it out, but performing operations in the subdirectory
would confusingly consider the in-tree ".git" directory as
the repository.

The regular git tools already make it hard to accidentally
add such an entry to a tree, and do not allow such entries
to enter the index at all. Teaching fsck about it provides
an additional safety check, and let's us avoid propagating
any such bogosity when transfer.fsckObjects is on.

Signed-off-by: Jeff King <peff@peff.net>
---
 fsck.c          |  5 +++++
 t/t1450-fsck.sh | 15 +++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/fsck.c b/fsck.c
index 31c9a51..99c0497 100644
--- a/fsck.c
+++ b/fsck.c
@@ -144,6 +144,7 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
 	int has_empty_name = 0;
 	int has_dot = 0;
 	int has_dotdot = 0;
+	int has_dotgit = 0;
 	int has_zero_pad = 0;
 	int has_bad_modes = 0;
 	int has_dup_entries = 0;
@@ -174,6 +175,8 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
 			has_dot = 1;
 		if (!strcmp(name, ".."))
 			has_dotdot = 1;
+		if (!strcmp(name, ".git"))
+			has_dotgit = 1;
 		has_zero_pad |= *(char *)desc.buffer == '0';
 		update_tree_entry(&desc);
 
@@ -227,6 +230,8 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
 		retval += error_func(&item->object, FSCK_WARN, "contains '.'");
 	if (has_dotdot)
 		retval += error_func(&item->object, FSCK_WARN, "contains '..'");
+	if (has_dotgit)
+		retval += error_func(&item->object, FSCK_WARN, "contains '.git'");
 	if (has_zero_pad)
 		retval += error_func(&item->object, FSCK_WARN, "contains zero-padded file modes");
 	if (has_bad_modes)
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 0b5c30b..d730734 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -253,4 +253,19 @@ test_expect_success 'fsck notices "." and ".." in trees' '
 	)
 '
 
+test_expect_success 'fsck notices ".git" in trees' '
+	(
+		git init dotgit &&
+		cd dotgit &&
+		blob=$(echo foo | git hash-object -w --stdin) &&
+		tab=$(printf "\\t") &&
+		git mktree <<-EOF &&
+		100644 blob $blob$tab.git
+		EOF
+		git fsck 2>out &&
+		cat out &&
+		grep "warning.*\\.git" out
+	)
+'
+
 test_done
-- 
1.8.0.207.gdf2154c

^ permalink raw reply related

* Re: [PATCH 0/5] jk/send-email-sender-prompt loose ends
From: Felipe Contreras @ 2012-11-28 21:37 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20121128182534.GA21020@sigill.intra.peff.net>

On Wed, Nov 28, 2012 at 7:25 PM, Jeff King <peff@peff.net> wrote:
> Here are the cleanups and refactorings split out from my
> jk/send-email-sender-prompt series. They can go right on master and are
> independent of Felipe's fc/send-email-no-sender-prompt topic.
>
>   [1/5]: test-lib: allow negation of prerequisites
>
> Same as before. I think this is a useful feature for the test suite (and
> 2/5 depends on it).
>
>   [2/5]: t7502: factor out autoident prerequisite
>
> Same as before. Patch 5/5 depends on it.
>
>   [3/5]: ident: make user_ident_explicitly_given static
>
> Same as before. Cleanup.
>
>   [4/5]: ident: keep separate "explicit" flags for author and committer
>
> Same as before. Cleanup. I do not know if anybody will ever care about
> the corner cases it fixes, so it is really just being defensive for
> future code.
>
>   [5/5]: t: add tests for "git var"
>
> Tests split out from he "git var can take multiple values" patch.
>
> Dropped were:
>
>   - "git var" can take multiple values. Nobody really cares about doing
>     so, and it's an external interface, so we'd have to support it
>     forever.
>
>   - exporting "explicit ident" flag via "git var"; same reasoning as
>     above
>
>   - Git.pm supporting explicit ident; ditto
>
>   - send-email prompting change; obsoleted by Felipe's patch

For what it's worth; they look good to me. However, I think it's worth
mentioning that there are no functional changes.

-- 
Felipe Contreras

^ permalink raw reply

* [PATCH v7 p1 00/13] fast-export and remote-testgit improvements
From: Felipe Contreras @ 2012-11-28 22:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras

Hi,

Basically the same as v6, except the last patch was dropped, and testgit
switched to '#!/usr/bin/env bash'.

Shouldn't break any tests now.

Felipe Contreras (13):
  fast-export: avoid importing blob marks
  remote-testgit: fix direction of marks
  remote-helpers: fix failure message
  Rename git-remote-testgit to git-remote-testpy
  Add new simplified git-remote-testgit
  remote-testgit: remove non-local functionality
  remote-testgit: remove irrelevant test
  remote-testgit: cleanup tests
  remote-testgit: exercise more features
  remote-testgit: report success after an import
  remote-testgit: implement the "done" feature manually
  fast-export: trivial cleanup
  fast-export: fix comparison in tests

 .gitignore                           |   2 +-
 Documentation/git-remote-testgit.txt |   2 +-
 Makefile                             |   2 +-
 builtin/fast-export.c                |   6 +-
 git-remote-testgit                   |  90 ++++++++++++
 git-remote-testgit.py                | 272 -----------------------------------
 git-remote-testpy.py                 | 272 +++++++++++++++++++++++++++++++++++
 git_remote_helpers/git/importer.py   |   2 +-
 t/t5800-remote-helpers.sh            | 148 -------------------
 t/t5800-remote-testpy.sh             | 148 +++++++++++++++++++
 t/t5801-remote-helpers.sh            | 165 +++++++++++++++++++++
 t/t9350-fast-export.sh               |  20 ++-
 12 files changed, 701 insertions(+), 428 deletions(-)
 create mode 100755 git-remote-testgit
 delete mode 100644 git-remote-testgit.py
 create mode 100644 git-remote-testpy.py
 delete mode 100755 t/t5800-remote-helpers.sh
 create mode 100755 t/t5800-remote-testpy.sh
 create mode 100755 t/t5801-remote-helpers.sh

-- 
1.8.0.1

^ permalink raw reply

* [PATCH v7 p1 02/13] remote-testgit: fix direction of marks
From: Felipe Contreras @ 2012-11-28 22:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>

Basically this is what we want:

  == pull ==

	testgit			transport-helper

	* export ->		import

	# testgit.marks		git.marks

  == push ==

	testgit			transport-helper

	* import		<- export

	# testgit.marks		git.marks

Each side should be agnostic of the other side. Because testgit.marks
(our helper marks) could be anything, not necessarily a format parsable
by fast-export or fast-import. In this test they happen to be compatible,
because we use those tools, but in the real world it would be something
completely different. For example, they might be mapping marks to
mercurial revisions (certainly not parsable by fast-import/export).

This is what we have:

  == pull ==

	testgit			transport-helper

	* export ->		import

	# testgit.marks		git.marks

  == push ==

	testgit			transport-helper

	* import		<- export

	# git.marks		testgit.marks

The only reason this is working is that git.marks and testgit.marks are
roughly the same.

This new behavior used to not be possible before due to a bug in
fast-export, but with the bug fixed, it works fine.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-remote-testgit.py              | 2 +-
 git_remote_helpers/git/importer.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-remote-testgit.py b/git-remote-testgit.py
index 5f3ebd2..ade797b 100644
--- a/git-remote-testgit.py
+++ b/git-remote-testgit.py
@@ -91,7 +91,7 @@ def do_capabilities(repo, args):
     if not os.path.exists(dirname):
         os.makedirs(dirname)
 
-    path = os.path.join(dirname, 'testgit.marks')
+    path = os.path.join(dirname, 'git.marks')
 
     print "*export-marks %s" % path
     if os.path.exists(path):
diff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py
index 5c6b595..e28cc8f 100644
--- a/git_remote_helpers/git/importer.py
+++ b/git_remote_helpers/git/importer.py
@@ -39,7 +39,7 @@ class GitImporter(object):
             gitdir = self.repo.gitpath
         else:
             gitdir = os.path.abspath(os.path.join(dirname, '.git'))
-        path = os.path.abspath(os.path.join(dirname, 'git.marks'))
+        path = os.path.abspath(os.path.join(dirname, 'testgit.marks'))
 
         if not os.path.exists(dirname):
             os.makedirs(dirname)
-- 
1.8.0.1

^ permalink raw reply related

* [PATCH v7 p1 01/13] fast-export: avoid importing blob marks
From: Felipe Contreras @ 2012-11-28 22:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>

We want to be able to import, and then export, using the same marks, so
that we don't push things that the other side already received.

Unfortunately, fast-export doesn't store blobs in the marks, but
fast-import does. This creates a mismatch when fast export is reusing a
mark that was previously stored by fast-import.

There is no point in one tool saving blobs, and the other not, but for
now let's just check in fast-export that the objects are indeed commits.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/fast-export.c  |  4 ++++
 t/t9350-fast-export.sh | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 12220ad..9b70ec1 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -614,6 +614,10 @@ static void import_marks(char *input_file)
 		if (object->flags & SHOWN)
 			error("Object %s already has a mark", sha1_to_hex(sha1));
 
+		if (object->type != OBJ_COMMIT)
+			/* only commits */
+			continue;
+
 		mark_object(object, mark);
 		if (last_idnum < mark)
 			last_idnum = mark;
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 3e821f9..5948b65 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -440,4 +440,18 @@ test_expect_success 'fast-export quotes pathnames' '
 	)
 '
 
+test_expect_success 'test bidirectionality' '
+	>marks-cur &&
+	>marks-new &&
+	git init marks-test &&
+	git fast-export --export-marks=marks-cur --import-marks=marks-cur --branches | \
+	git --git-dir=marks-test/.git fast-import --export-marks=marks-new --import-marks=marks-new &&
+	(cd marks-test &&
+	git reset --hard &&
+	echo Wohlauf > file &&
+	git commit -a -m "back in time") &&
+	git --git-dir=marks-test/.git fast-export --export-marks=marks-new --import-marks=marks-new --branches | \
+	git fast-import --export-marks=marks-cur --import-marks=marks-cur
+'
+
 test_done
-- 
1.8.0.1

^ permalink raw reply related

* [PATCH v7 p1 03/13] remote-helpers: fix failure message
From: Felipe Contreras @ 2012-11-28 22:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>

This is remote-testgit, not remote-hg.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5800-remote-helpers.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh
index e7dc668..d46fa40 100755
--- a/t/t5800-remote-helpers.sh
+++ b/t/t5800-remote-helpers.sh
@@ -8,7 +8,7 @@ test_description='Test remote-helper import and export commands'
 . ./test-lib.sh
 
 if ! test_have_prereq PYTHON ; then
-	skip_all='skipping git-remote-hg tests, python not available'
+	skip_all='skipping remote-testgit tests, python not available'
 	test_done
 fi
 
@@ -17,7 +17,7 @@ import sys
 if sys.hexversion < 0x02040000:
     sys.exit(1)
 ' || {
-	skip_all='skipping git-remote-hg tests, python version < 2.4'
+	skip_all='skipping remote-testgit tests, python version < 2.4'
 	test_done
 }
 
-- 
1.8.0.1

^ permalink raw reply related

* [PATCH v7 p1 05/13] Add new simplified git-remote-testgit
From: Felipe Contreras @ 2012-11-28 22:11 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>

Exercising the python remote helper framework is for another tool and
another test. This is about testing the remote-helper interface.

It's way simpler, it exercises the same features of remote helpers, it's
easy to read and understand, and it doesn't depend on python.

For now let's just copy the old remote-helpers test script, although
some of those tests don't make sense. In addition, this script would be
able to test other features not currently being tested.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-remote-testgit.txt |   2 +-
 git-remote-testgit                   |  64 ++++++++++++++++
 t/t5801-remote-helpers.sh            | 139 +++++++++++++++++++++++++++++++++++
 3 files changed, 204 insertions(+), 1 deletion(-)
 create mode 100755 git-remote-testgit
 create mode 100755 t/t5801-remote-helpers.sh

diff --git a/Documentation/git-remote-testgit.txt b/Documentation/git-remote-testgit.txt
index 2a67d45..612a625 100644
--- a/Documentation/git-remote-testgit.txt
+++ b/Documentation/git-remote-testgit.txt
@@ -19,7 +19,7 @@ testcase for the remote-helper functionality, and as an example to
 show remote-helper authors one possible implementation.
 
 The best way to learn more is to read the comments and source code in
-'git-remote-testgit.py'.
+'git-remote-testgit'.
 
 SEE ALSO
 --------
diff --git a/git-remote-testgit b/git-remote-testgit
new file mode 100755
index 0000000..5042f5a
--- /dev/null
+++ b/git-remote-testgit
@@ -0,0 +1,64 @@
+#!/usr/bin/env bash
+# Copyright (c) 2012 Felipe Contreras
+
+alias=$1
+url=$2
+
+# huh?
+url="${url#file://}"
+
+dir="$GIT_DIR/testgit/$alias"
+prefix="refs/testgit/$alias"
+refspec="refs/heads/*:${prefix}/heads/*"
+
+gitmarks="$dir/git.marks"
+testgitmarks="$dir/testgit.marks"
+
+export GIT_DIR="$url/.git"
+
+mkdir -p "$dir"
+
+test -e "$gitmarks" || > "$gitmarks"
+test -e "$testgitmarks" || > "$testgitmarks"
+
+while read line
+do
+	case $line in
+	capabilities)
+		echo 'import'
+		echo 'export'
+		echo "refspec $refspec"
+		echo "*import-marks $gitmarks"
+		echo "*export-marks $gitmarks"
+		echo
+		;;
+	list)
+		git for-each-ref --format='? %(refname)' 'refs/heads/'
+		head=$(git symbolic-ref HEAD)
+		echo "@$head HEAD"
+		echo
+		;;
+	import*)
+		# read all import lines
+		while true
+		do
+			ref="${line#* }"
+			refs="$refs $ref"
+			read line
+			test "${line%% *}" != "import" && break
+		done
+
+		echo "feature import-marks=$gitmarks"
+		echo "feature export-marks=$gitmarks"
+		git fast-export --use-done-feature --{import,export}-marks="$testgitmarks" $refs |
+		sed -e "s#refs/heads/#${prefix}/heads/#g"
+		;;
+	export)
+		git fast-import --{import,export}-marks="$testgitmarks" --quiet
+		echo
+		;;
+	'')
+		exit
+		;;
+	esac
+done
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
new file mode 100755
index 0000000..f52ab14
--- /dev/null
+++ b/t/t5801-remote-helpers.sh
@@ -0,0 +1,139 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Sverre Rabbelier
+#
+
+test_description='Test remote-helper import and export commands'
+
+. ./test-lib.sh
+
+if ! type "${BASH-bash}" >/dev/null 2>&1; then
+	skip_all='skipping remote-testgit tests, bash not available'
+	test_done
+fi
+
+compare_refs() {
+	git --git-dir="$1/.git" rev-parse --verify $2 >expect &&
+	git --git-dir="$3/.git" rev-parse --verify $4 >actual &&
+	test_cmp expect actual
+}
+
+test_expect_success 'setup repository' '
+	git init --bare server/.git &&
+	git clone server public &&
+	(cd public &&
+	 echo content >file &&
+	 git add file &&
+	 git commit -m one &&
+	 git push origin master)
+'
+
+test_expect_success 'cloning from local repo' '
+	git clone "testgit::${PWD}/server" localclone &&
+	test_cmp public/file localclone/file
+'
+
+test_expect_success 'cloning from remote repo' '
+	git clone "testgit::file://${PWD}/server" clone &&
+	test_cmp public/file clone/file
+'
+
+test_expect_success 'create new commit on remote' '
+	(cd public &&
+	 echo content >>file &&
+	 git commit -a -m two &&
+	 git push)
+'
+
+test_expect_success 'pulling from local repo' '
+	(cd localclone && git pull) &&
+	test_cmp public/file localclone/file
+'
+
+test_expect_success 'pulling from remote remote' '
+	(cd clone && git pull) &&
+	test_cmp public/file clone/file
+'
+
+test_expect_success 'pushing to local repo' '
+	(cd localclone &&
+	echo content >>file &&
+	git commit -a -m three &&
+	git push) &&
+	compare_refs localclone HEAD server HEAD
+'
+
+# Generally, skip this test.  It demonstrates a now-fixed race in
+# git-remote-testgit, but is too slow to leave in for general use.
+: test_expect_success 'racily pushing to local repo' '
+	test_when_finished "rm -rf server2 localclone2" &&
+	cp -R server server2 &&
+	git clone "testgit::${PWD}/server2" localclone2 &&
+	(cd localclone2 &&
+	echo content >>file &&
+	git commit -a -m three &&
+	GIT_REMOTE_TESTGIT_SLEEPY=2 git push) &&
+	compare_refs localclone2 HEAD server2 HEAD
+'
+
+test_expect_success 'synch with changes from localclone' '
+	(cd clone &&
+	 git pull)
+'
+
+test_expect_success 'pushing remote local repo' '
+	(cd clone &&
+	echo content >>file &&
+	git commit -a -m four &&
+	git push) &&
+	compare_refs clone HEAD server HEAD
+'
+
+test_expect_success 'fetch new branch' '
+	(cd public &&
+	 git checkout -b new &&
+	 echo content >>file &&
+	 git commit -a -m five &&
+	 git push origin new
+	) &&
+	(cd localclone &&
+	 git fetch origin new
+	) &&
+	compare_refs public HEAD localclone FETCH_HEAD
+'
+
+test_expect_success 'fetch multiple branches' '
+	(cd localclone &&
+	 git fetch
+	) &&
+	compare_refs server master localclone refs/remotes/origin/master &&
+	compare_refs server new localclone refs/remotes/origin/new
+'
+
+test_expect_success 'push when remote has extra refs' '
+	(cd clone &&
+	 echo content >>file &&
+	 git commit -a -m six &&
+	 git push
+	) &&
+	compare_refs clone master server master
+'
+
+test_expect_success 'push new branch by name' '
+	(cd clone &&
+	 git checkout -b new-name  &&
+	 echo content >>file &&
+	 git commit -a -m seven &&
+	 git push origin new-name
+	) &&
+	compare_refs clone HEAD server refs/heads/new-name
+'
+
+test_expect_failure 'push new branch with old:new refspec' '
+	(cd clone &&
+	 git push origin new-name:new-refspec
+	) &&
+	compare_refs clone HEAD server refs/heads/new-refspec
+'
+
+test_done
-- 
1.8.0.1

^ permalink raw reply related

* [PATCH v7 p1 06/13] remote-testgit: remove non-local functionality
From: Felipe Contreras @ 2012-11-28 22:11 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>

This only makes sense for the python remote helpers framework. The tests
don't exercise any feature of transport helper. Remove them.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-remote-testgit        |  3 ---
 t/t5801-remote-helpers.sh | 50 ++++++++++++++++++++---------------------------
 2 files changed, 21 insertions(+), 32 deletions(-)

diff --git a/git-remote-testgit b/git-remote-testgit
index 5042f5a..5117ab5 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -4,9 +4,6 @@
 alias=$1
 url=$2
 
-# huh?
-url="${url#file://}"
-
 dir="$GIT_DIR/testgit/$alias"
 prefix="refs/testgit/$alias"
 refspec="refs/heads/*:${prefix}/heads/*"
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index f52ab14..2f7fc10 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -33,11 +33,6 @@ test_expect_success 'cloning from local repo' '
 	test_cmp public/file localclone/file
 '
 
-test_expect_success 'cloning from remote repo' '
-	git clone "testgit::file://${PWD}/server" clone &&
-	test_cmp public/file clone/file
-'
-
 test_expect_success 'create new commit on remote' '
 	(cd public &&
 	 echo content >>file &&
@@ -50,11 +45,6 @@ test_expect_success 'pulling from local repo' '
 	test_cmp public/file localclone/file
 '
 
-test_expect_success 'pulling from remote remote' '
-	(cd clone && git pull) &&
-	test_cmp public/file clone/file
-'
-
 test_expect_success 'pushing to local repo' '
 	(cd localclone &&
 	echo content >>file &&
@@ -76,19 +66,6 @@ test_expect_success 'pushing to local repo' '
 	compare_refs localclone2 HEAD server2 HEAD
 '
 
-test_expect_success 'synch with changes from localclone' '
-	(cd clone &&
-	 git pull)
-'
-
-test_expect_success 'pushing remote local repo' '
-	(cd clone &&
-	echo content >>file &&
-	git commit -a -m four &&
-	git push) &&
-	compare_refs clone HEAD server HEAD
-'
-
 test_expect_success 'fetch new branch' '
 	(cd public &&
 	 git checkout -b new &&
@@ -102,6 +79,20 @@ test_expect_success 'fetch new branch' '
 	compare_refs public HEAD localclone FETCH_HEAD
 '
 
+#
+# This is only needed because of a bug not detected by this script. It will be
+# fixed shortly, but for now lets not cause regressions.
+#
+test_expect_success 'bump commit in public' '
+	(cd public &&
+	git checkout master &&
+	git pull &&
+	echo content >>file &&
+	git commit -a -m four &&
+	git push) &&
+	compare_refs public HEAD server HEAD
+'
+
 test_expect_success 'fetch multiple branches' '
 	(cd localclone &&
 	 git fetch
@@ -111,29 +102,30 @@ test_expect_success 'fetch multiple branches' '
 '
 
 test_expect_success 'push when remote has extra refs' '
-	(cd clone &&
+	(cd localclone &&
+	 git reset --hard origin/master &&
 	 echo content >>file &&
 	 git commit -a -m six &&
 	 git push
 	) &&
-	compare_refs clone master server master
+	compare_refs localclone master server master
 '
 
 test_expect_success 'push new branch by name' '
-	(cd clone &&
+	(cd localclone &&
 	 git checkout -b new-name  &&
 	 echo content >>file &&
 	 git commit -a -m seven &&
 	 git push origin new-name
 	) &&
-	compare_refs clone HEAD server refs/heads/new-name
+	compare_refs localclone HEAD server refs/heads/new-name
 '
 
 test_expect_failure 'push new branch with old:new refspec' '
-	(cd clone &&
+	(cd localclone &&
 	 git push origin new-name:new-refspec
 	) &&
-	compare_refs clone HEAD server refs/heads/new-refspec
+	compare_refs localclone HEAD server refs/heads/new-refspec
 '
 
 test_done
-- 
1.8.0.1

^ permalink raw reply related

* [PATCH v7 p1 07/13] remote-testgit: remove irrelevant test
From: Felipe Contreras @ 2012-11-28 22:11 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>

This was only to cover a bug that was fixed in remote-testpy not to
resurface.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5801-remote-helpers.sh | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 2f7fc10..6801529 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -53,19 +53,6 @@ test_expect_success 'pushing to local repo' '
 	compare_refs localclone HEAD server HEAD
 '
 
-# Generally, skip this test.  It demonstrates a now-fixed race in
-# git-remote-testgit, but is too slow to leave in for general use.
-: test_expect_success 'racily pushing to local repo' '
-	test_when_finished "rm -rf server2 localclone2" &&
-	cp -R server server2 &&
-	git clone "testgit::${PWD}/server2" localclone2 &&
-	(cd localclone2 &&
-	echo content >>file &&
-	git commit -a -m three &&
-	GIT_REMOTE_TESTGIT_SLEEPY=2 git push) &&
-	compare_refs localclone2 HEAD server2 HEAD
-'
-
 test_expect_success 'fetch new branch' '
 	(cd public &&
 	 git checkout -b new &&
-- 
1.8.0.1

^ permalink raw reply related

* [PATCH v7 p1 08/13] remote-testgit: cleanup tests
From: Felipe Contreras @ 2012-11-28 22:11 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>

We don't need a bare 'server' and an intermediary 'public'. The repos
can talk to each other directly; that's what we want to exercise.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5801-remote-helpers.sh | 63 ++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 34 deletions(-)

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 6801529..bc0b5f7 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -19,100 +19,95 @@ compare_refs() {
 }
 
 test_expect_success 'setup repository' '
-	git init --bare server/.git &&
-	git clone server public &&
-	(cd public &&
+	git init server &&
+	(cd server &&
 	 echo content >file &&
 	 git add file &&
-	 git commit -m one &&
-	 git push origin master)
+	 git commit -m one)
 '
 
 test_expect_success 'cloning from local repo' '
-	git clone "testgit::${PWD}/server" localclone &&
-	test_cmp public/file localclone/file
+	git clone "testgit::${PWD}/server" local &&
+	test_cmp server/file local/file
 '
 
 test_expect_success 'create new commit on remote' '
-	(cd public &&
+	(cd server &&
 	 echo content >>file &&
-	 git commit -a -m two &&
-	 git push)
+	 git commit -a -m two)
 '
 
 test_expect_success 'pulling from local repo' '
-	(cd localclone && git pull) &&
-	test_cmp public/file localclone/file
+	(cd local && git pull) &&
+	test_cmp server/file local/file
 '
 
 test_expect_success 'pushing to local repo' '
-	(cd localclone &&
+	(cd local &&
 	echo content >>file &&
 	git commit -a -m three &&
 	git push) &&
-	compare_refs localclone HEAD server HEAD
+	compare_refs local HEAD server HEAD
 '
 
 test_expect_success 'fetch new branch' '
-	(cd public &&
+	(cd server &&
+	 git reset --hard &&
 	 git checkout -b new &&
 	 echo content >>file &&
-	 git commit -a -m five &&
-	 git push origin new
+	 git commit -a -m five
 	) &&
-	(cd localclone &&
+	(cd local &&
 	 git fetch origin new
 	) &&
-	compare_refs public HEAD localclone FETCH_HEAD
+	compare_refs server HEAD local FETCH_HEAD
 '
 
 #
 # This is only needed because of a bug not detected by this script. It will be
 # fixed shortly, but for now lets not cause regressions.
 #
-test_expect_success 'bump commit in public' '
-	(cd public &&
+test_expect_success 'bump commit in server' '
+	(cd server &&
 	git checkout master &&
-	git pull &&
 	echo content >>file &&
-	git commit -a -m four &&
-	git push) &&
-	compare_refs public HEAD server HEAD
+	git commit -a -m four) &&
+	compare_refs server HEAD server HEAD
 '
 
 test_expect_success 'fetch multiple branches' '
-	(cd localclone &&
+	(cd local &&
 	 git fetch
 	) &&
-	compare_refs server master localclone refs/remotes/origin/master &&
-	compare_refs server new localclone refs/remotes/origin/new
+	compare_refs server master local refs/remotes/origin/master &&
+	compare_refs server new local refs/remotes/origin/new
 '
 
 test_expect_success 'push when remote has extra refs' '
-	(cd localclone &&
+	(cd local &&
 	 git reset --hard origin/master &&
 	 echo content >>file &&
 	 git commit -a -m six &&
 	 git push
 	) &&
-	compare_refs localclone master server master
+	compare_refs local master server master
 '
 
 test_expect_success 'push new branch by name' '
-	(cd localclone &&
+	(cd local &&
 	 git checkout -b new-name  &&
 	 echo content >>file &&
 	 git commit -a -m seven &&
 	 git push origin new-name
 	) &&
-	compare_refs localclone HEAD server refs/heads/new-name
+	compare_refs local HEAD server refs/heads/new-name
 '
 
 test_expect_failure 'push new branch with old:new refspec' '
-	(cd localclone &&
+	(cd local &&
 	 git push origin new-name:new-refspec
 	) &&
-	compare_refs localclone HEAD server refs/heads/new-refspec
+	compare_refs local HEAD server refs/heads/new-refspec
 '
 
 test_done
-- 
1.8.0.1

^ permalink raw reply related

* [PATCH v7 p1 10/13] remote-testgit: report success after an import
From: Felipe Contreras @ 2012-11-28 22:11 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>

Doesn't make a difference for the tests, but it does for the ones
seeking reference.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-remote-testgit | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/git-remote-testgit b/git-remote-testgit
index efda74b..6fb8780 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -65,7 +65,20 @@ do
 		sed -e "s#refs/heads/#${prefix}/heads/#g"
 		;;
 	export)
+		before=$(git for-each-ref --format='%(refname) %(objectname)')
+
 		git fast-import "${testgitmarks_args[@]}" --quiet
+
+		after=$(git for-each-ref --format='%(refname) %(objectname)')
+
+		# figure out which refs were updated
+		join -e 0 -o '0 1.2 2.2' -a 2 <(echo "$before") <(echo "$after") |
+		while read ref a b
+		do
+			test $a == $b && continue
+			echo "ok $ref"
+		done
+
 		echo
 		;;
 	'')
-- 
1.8.0.1

^ permalink raw reply related

* [PATCH v7 p1 09/13] remote-testgit: exercise more features
From: Felipe Contreras @ 2012-11-28 22:11 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>

Unfortunately a lot of these tests fail.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-remote-testgit        | 38 +++++++++++++++++++++++-----------
 t/t5801-remote-helpers.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 12 deletions(-)

diff --git a/git-remote-testgit b/git-remote-testgit
index 5117ab5..efda74b 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -6,17 +6,25 @@ url=$2
 
 dir="$GIT_DIR/testgit/$alias"
 prefix="refs/testgit/$alias"
-refspec="refs/heads/*:${prefix}/heads/*"
 
-gitmarks="$dir/git.marks"
-testgitmarks="$dir/testgit.marks"
+default_refspec="refs/heads/*:${prefix}/heads/*"
+
+refspec="${GIT_REMOTE_TESTGIT_REFSPEC-$default_refspec}"
+
+test -z "$refspec" && prefix="refs"
 
 export GIT_DIR="$url/.git"
 
 mkdir -p "$dir"
 
-test -e "$gitmarks" || > "$gitmarks"
-test -e "$testgitmarks" || > "$testgitmarks"
+if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"
+then
+	gitmarks="$dir/git.marks"
+	testgitmarks="$dir/testgit.marks"
+	test -e "$gitmarks" || >"$gitmarks"
+	test -e "$testgitmarks" || >"$testgitmarks"
+	testgitmarks_args=( "--"{import,export}"-marks=$testgitmarks" )
+fi
 
 while read line
 do
@@ -24,9 +32,12 @@ do
 	capabilities)
 		echo 'import'
 		echo 'export'
-		echo "refspec $refspec"
-		echo "*import-marks $gitmarks"
-		echo "*export-marks $gitmarks"
+		test -n "$refspec" && echo "refspec $refspec"
+		if test -n "$gitmarks"
+		then
+			echo "*import-marks $gitmarks"
+			echo "*export-marks $gitmarks"
+		fi
 		echo
 		;;
 	list)
@@ -45,13 +56,16 @@ do
 			test "${line%% *}" != "import" && break
 		done
 
-		echo "feature import-marks=$gitmarks"
-		echo "feature export-marks=$gitmarks"
-		git fast-export --use-done-feature --{import,export}-marks="$testgitmarks" $refs |
+		if test -n "$gitmarks"
+		then
+			echo "feature import-marks=$gitmarks"
+			echo "feature export-marks=$gitmarks"
+		fi
+		git fast-export --use-done-feature "${testgitmarks_args[@]}" $refs |
 		sed -e "s#refs/heads/#${prefix}/heads/#g"
 		;;
 	export)
-		git fast-import --{import,export}-marks="$testgitmarks" --quiet
+		git fast-import "${testgitmarks_args[@]}" --quiet
 		echo
 		;;
 	'')
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index bc0b5f7..12ae256 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -110,4 +110,56 @@ test_expect_failure 'push new branch with old:new refspec' '
 	compare_refs local HEAD server refs/heads/new-refspec
 '
 
+test_expect_success 'cloning without refspec' '
+	GIT_REMOTE_TESTGIT_REFSPEC="" \
+	git clone "testgit::${PWD}/server" local2 &&
+	compare_refs local2 HEAD server HEAD
+'
+
+test_expect_success 'pulling without refspecs' '
+	(cd local2 &&
+	git reset --hard &&
+	GIT_REMOTE_TESTGIT_REFSPEC="" git pull) &&
+	compare_refs local2 HEAD server HEAD
+'
+
+test_expect_failure 'pushing without refspecs' '
+	test_when_finished "(cd local2 && git reset --hard origin)" &&
+	(cd local2 &&
+	echo content >>file &&
+	git commit -a -m ten &&
+	GIT_REMOTE_TESTGIT_REFSPEC="" git push) &&
+	compare_refs local2 HEAD server HEAD
+'
+
+test_expect_success 'pulling with straight refspec' '
+	(cd local2 &&
+	GIT_REMOTE_TESTGIT_REFSPEC="*:*" git pull) &&
+	compare_refs local2 HEAD server HEAD
+'
+
+test_expect_failure 'pushing with straight refspec' '
+	test_when_finished "(cd local2 && git reset --hard origin)" &&
+	(cd local2 &&
+	echo content >>file &&
+	git commit -a -m eleven &&
+	GIT_REMOTE_TESTGIT_REFSPEC="*:*" git push) &&
+	compare_refs local2 HEAD server HEAD
+'
+
+test_expect_success 'pulling without marks' '
+	(cd local2 &&
+	GIT_REMOTE_TESTGIT_NO_MARKS=1 git pull) &&
+	compare_refs local2 HEAD server HEAD
+'
+
+test_expect_failure 'pushing without marks' '
+	test_when_finished "(cd local2 && git reset --hard origin)" &&
+	(cd local2 &&
+	echo content >>file &&
+	git commit -a -m twelve &&
+	GIT_REMOTE_TESTGIT_NO_MARKS=1 git push) &&
+	compare_refs local2 HEAD server HEAD
+'
+
 test_done
-- 
1.8.0.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox