Git development
 help / color / mirror / Atom feed
* Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
From: Jeff King @ 2012-11-15 10:43 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <CAMP44s2NBGDRLUKhBTU+kNy7Fyn8T6qm3nneSbS4rrNN1oPgdw@mail.gmail.com>

On Thu, Nov 15, 2012 at 11:28:46AM +0100, Felipe Contreras wrote:

> I tried both:
> 
> ok 19 # skip implicit ident prompts for sender (missing AUTOIDENT of
> PERL,AUTOIDENT)
> ok 20 - broken implicit ident aborts send-email
> 
> ok 19 - implicit ident prompts for sender
> ok 20 # skip broken implicit ident aborts send-email (missing
> !AUTOIDENT of PERL,!AUTOIDENT)
> 
> However, it would be much easier if ident learned to check
> GIT_TEST_FAKE_HOSTNAME, or something.

Yes, it would be. It has two downsides:

  1. The regular git code has to be instrumented to respect the
     variable, so it can potentially affect git in production use
     outside of the test suite. Since such code is simple, though, it is
     probably not a big risk.

  2. We would not actually exercise the code paths for doing
     hostname and GECOS lookup. We do not test their resulting values,
     so the coverage is not great now, but we do at least run the code,
     which would let a run with "--valgrind" check it. I guess we could
     go through the motions of assembling the ident and then replace
     it at the end with the fake value.

I don't have a strong opinion either way.

> > One whose system is configured in such a way that git can produce an
> > automatic ident (i.e., has a non-blank GECOS name and a FQDN).
> 
> And doesn't have any of the following:
> 
>  * configured user.name/user.email
>  * specified $EMAIL
>  * configured sendemail.from
>  * specified --from argument
> 
> Very unlikely.

That is certainly the opinion you have stated already. I'm not sure I
agree. Linus, for example, was an advocate of such a configuration early
on in git's history. I don't think he still runs that way, though.

> And then, what would be the consequences of not receiving this prompt?

An email would be sent with the generated identity.

-Peff

^ permalink raw reply

* Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
From: Felipe Contreras @ 2012-11-15 10:28 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121115083315.GA23377@sigill.intra.peff.net>

On Thu, Nov 15, 2012 at 9:33 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Nov 15, 2012 at 03:08:42AM +0100, Felipe Contreras wrote:
>
>> I don't think there's any need for all that, this does the trick:
>>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index aea66a0..503e551 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -748,16 +748,11 @@ if (!$force) {
>>         }
>>  }
>>
>> -my $prompting = 0;
>>  if (!defined $sender) {
>>         $sender = $repoauthor || $repocommitter || '';
>> -       $sender = ask("Who should the emails appear to be from? [$sender] ",
>> -                     default => $sender,
>> -                     valid_re => qr/\@.*\./, confirm_only => 1);
>> -       print "Emails will be sent from: ", $sender, "\n";
>> -       $prompting++;
>>  }
>>
>> +my $prompting = 0;
>>
>> This passes all the current tests and the ones you added.
>
> It may pass on your system, but it will not on a system that meets the
> AUTOIDENT prerequisite (it fails the new t9001.19 on my system; I
> suspect your system config is such that we skip t9001.19 and run
> t9001.20, whereas mine is the opposite).

I tried both:

ok 19 # skip implicit ident prompts for sender (missing AUTOIDENT of
PERL,AUTOIDENT)
ok 20 - broken implicit ident aborts send-email

ok 19 - implicit ident prompts for sender
ok 20 # skip broken implicit ident aborts send-email (missing
!AUTOIDENT of PERL,!AUTOIDENT)

However, it would be much easier if ident learned to check
GIT_TEST_FAKE_HOSTNAME, or something.

But then I realized I had to run 'make' again. Yes, my patch breaks
test 19, but see below.

>> Which kind of user will get the prompt with your patch, that would
>> miss it with mine?
>
> One whose system is configured in such a way that git can produce an
> automatic ident (i.e., has a non-blank GECOS name and a FQDN).

And doesn't have any of the following:

 * configured user.name/user.email
 * specified $EMAIL
 * configured sendemail.from
 * specified --from argument

Very unlikely. And then, what would be the consequences of not
receiving this prompt?

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* use cases for git namespaces
From: Sitaram Chamarty @ 2012-11-15  8:33 UTC (permalink / raw)
  To: Git Mailing List

Hi,

It seems to me that whatever namespaces can do, can functionally be
done using just a subdirectory of branches.   The only real
differences I can see are (a) a client sees less branch clutter, and
(b) a fetch/clone pulls down less if the big stuff is in another
namespace.

I would like to understand what other uses/reasons were thought of.

I looked for discussion on the ml archives.  I found the patch series
but could not easily find much *discussion* of the feature and its
design.  I found one post [1] that indicated that "part of the
rationale..." (being what I described above), but I would like to
understand the *rest* of the rationale.

Pointers to gmane are also fine, or brief descriptions of uses [being]
made of this.

[1]: http://article.gmane.org/gmane.comp.version-control.git/175832/match=namespace

Thanks

-- 
Sitaram

^ permalink raw reply

* Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
From: Jeff King @ 2012-11-15  8:33 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <CAMP44s0d+g7bXCnOf55jZNNFS6uJ+4BDowx5uYxWBP4xA+-0zA@mail.gmail.com>

On Thu, Nov 15, 2012 at 03:08:42AM +0100, Felipe Contreras wrote:

> I don't think there's any need for all that, this does the trick:
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index aea66a0..503e551 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -748,16 +748,11 @@ if (!$force) {
>         }
>  }
> 
> -my $prompting = 0;
>  if (!defined $sender) {
>         $sender = $repoauthor || $repocommitter || '';
> -       $sender = ask("Who should the emails appear to be from? [$sender] ",
> -                     default => $sender,
> -                     valid_re => qr/\@.*\./, confirm_only => 1);
> -       print "Emails will be sent from: ", $sender, "\n";
> -       $prompting++;
>  }
> 
> +my $prompting = 0;
> 
> This passes all the current tests and the ones you added.

It may pass on your system, but it will not on a system that meets the
AUTOIDENT prerequisite (it fails the new t9001.19 on my system; I
suspect your system config is such that we skip t9001.19 and run
t9001.20, whereas mine is the opposite).

> Which kind of user will get the prompt with your patch, that would
> miss it with mine?

One whose system is configured in such a way that git can produce an
automatic ident (i.e., has a non-blank GECOS name and a FQDN).

-Peff

^ permalink raw reply

* Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
From: Jonathan Nieder @ 2012-11-15  8:23 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Felipe Contreras, Thomas Rast, Junio C Hamano
In-Reply-To: <20121115003640.GH17819@sigill.intra.peff.net>

Jeff King wrote:

> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -191,15 +191,47 @@ test_expect_success $PREREQ 'Show all headers' '
>  
>  test_expect_success $PREREQ 'Prompting works' '
>  	clean_fake_sendmail &&
> -	(echo "Example <from@example.com>"
> -	 echo "to@example.com"
> +	(echo "to@example.com"
>  	 echo ""
>  	) | GIT_SEND_EMAIL_NOTTY=1 git send-email \
>  		--smtp-server="$(pwd)/fake.sendmail" \
>  		$patches \
>  		2>errors &&
> +		grep "^From: A U Thor <author@example.com>\$" msgtxt1 &&
> +		grep "^To: to@example.com\$" msgtxt1
> +'

The indentation seems strange here --- are the new "grep" lines
continuations of the git send-email line?

It's probably easier to change the structure completely:

	clean_fake_sendmail &&
	echo to@examples.com >prompt.input &&
	echo >>prompt.input &&
	GIT_SEND_EMAIL_NOTTY=1 \
		git send-email --smtp-server=... $patches <prompt.input &&
	grep "^From: A U Thor <authorident@example.com>\$" msgtxt1 &&
	grep "^To: to@example.com\$" msgtxt1

> +test_expect_success $PREREQ,AUTOIDENT 'implicit ident prompts for sender' '
> +	clean_fake_sendmail &&
> +	(echo "Example <from@example.com>" &&
> +	 echo "to@example.com" &&
> +	 echo ""
> +	) |
> +	(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" \
> +		$patches \
> +		2>errors &&
>  		grep "^From: Example <from@example.com>\$" msgtxt1 &&
>  		grep "^To: to@example.com\$" msgtxt1
> +	)
> +'

Likewise:

	clean_fake_sendmail &&
	echo "Example <from@example.com>" >prompt.in &&
	echo to@example.com >>prompt.in
	echo >>prompt.in &&
	(
		sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&
		sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&
		GIT_SEND_EMAIL_NOTTY=1 \
			git send-email --smtp-server=... $patches <prompt.in
	) &&
	grep "^From: Example <from@example.com>\$" msgtxt1 &&
	grep "^To: to@example.com\$" msgtxt1

> +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.out &&
> +		test_i18ngrep "tell me who you are" errors.out
> +	)
>  '

Likewise:

	clean_fake_sendmail &&
	(
		sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&
		sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&
		GIT_SEND_EMAIL_NOTTY=1 \
			git send-email --smtp-server=... $patches </dev/null 2>err
	) &&
	test_i18ngrep "[Tt]ell me who you are" err

For what it's worth, with or without such changes,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>

^ permalink raw reply

* Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Michael Haggerty @ 2012-11-15  8:18 UTC (permalink / raw)
  To: David Aguilar
  Cc: Junio C Hamano, Jiang Xin, Lea Wiemann, David Reiss,
	Johannes Sixt, git, Lars R. Damerow, Jeff King, Marc Jordan
In-Reply-To: <CAJDDKr5F5EcXaTuPWgE5MZJQ=Of6MwW+RmRhhXOLyfQzanjEwQ@mail.gmail.com>

On 11/13/2012 09:50 PM, David Aguilar wrote:
> On Mon, Nov 12, 2012 at 9:47 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>>
>>> The log message of the original commit (0454dd93bf) described the
>>> following scenario: a /home partition under which user home directories
>>> are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid
>>> hitting /home/.git, /home/.git/objects, and /home/objects (which would
>>> attempt to automount those directories).  I believe that this scenario
>>> would not be slowed down by my patches.
>>>
>>> How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a
>>> slowdown?
>>
>> Yeah, I was also wondering about that.
>>
>> David?
> 
> I double-checked our configuration and all the parent directories
> of those listed in GIT_CEILING_DIRECTORIES are local,
> so our particular configuration would not have a performance hit.
> 
> We do have multiple directories listed there.  Some of them share
> a parent directory.  I'm assuming the implementation is simple and
> does not try and avoid repeating the check when the parent dir is
> the same across multiple entries.
> 
> In any case, it won't be a problem in practice based on my
> reading of the current code.

OK, so we're back to the following status: some people (including me)
are nervous that this change could cause a performance regression,
though it seems that the most sensible ways of using the
GIT_CEILING_DIRECTORIES feature would not be affected.

In favor: Currently, if a directory containing a symlink is added to
GIT_CEILING_DIRECTORIES, then GIT_CEILING_DIRECTORIES will not work, git
has no way of recognizing that there is a problem, and the only symptom
observable by the user is that the hoped-for performance improvement
from using GIT_CEILING_DIRECTORIES will not materialize (or will
disappear after a filesystem reorg) [1].

Against: The change will cause a performance regression if a
slow-to-stat directory is listed in GIT_CEILING_DIRECTORIES.  The
slowdown will occur whenever git is run outside of a true git-managed
project, most nastily in the case of using __git_ps1 in a shell prompt.

I don't have a preference either way about whether these patches should
be merged.

Michael

[1] It is also conceivable that GIT_CEILING_DIRECTORIES is being used to
*hide* an enclosing git project rather than to inform git that there are
no enclosing projects, in which case the enclosing project would *not*
be hidden.  This is in fact the mechanism by which the problem causes
failures in our test suite.  But I don't expect that this is a common
real-world scenario, and anyway such a failure would be obvious to the
user and quickly fixed.

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: [PATCHv2 7/8] Git.pm: teach "ident" to query explicitness
From: Jonathan Nieder @ 2012-11-15  8:13 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Felipe Contreras, Thomas Rast, Junio C Hamano
In-Reply-To: <20121115003628.GG17819@sigill.intra.peff.net>

Jeff King wrote:

> Signed-off-by: Jeff King <peff@peff.net>

For what it's worth,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>

^ permalink raw reply

* Re: [PATCHv2 5/8] var: accept multiple variables on the command line
From: Jonathan Nieder @ 2012-11-15  8:10 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Felipe Contreras, Thomas Rast, Junio C Hamano
In-Reply-To: <20121115003504.GE17819@sigill.intra.peff.net>

Jeff King wrote:

> This patch lets callers specify multiple variables, and
> prints one per line.
[...]
> Signed-off-by: Jeff King <peff@peff.net>

Very pleasantly done --- thanks.  For what it's worth, assuming this
is tested, I can't see any reason not to apply it.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

^ permalink raw reply

* Re: [PATCHv2 4/8] ident: keep separate "explicit" flags for author and committer
From: Jonathan Nieder @ 2012-11-15  8:04 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Felipe Contreras, Thomas Rast, Junio C Hamano
In-Reply-To: <20121115003413.GD17819@sigill.intra.peff.net>

Jeff King wrote:

>   1. GIT_COMMITTER_* is set explicitly, but we fallback for
>      GIT_AUTHOR. We claim the ident is explicit, even though
>      the author is not.
>
>   2. GIT_AUTHOR_* is set and we ask for author ident, but
>      not committer ident. We will claim the ident is
>      implicit, even though it is explicit.
>
> This patch uses two variables instead of one, updates both
> when we set the "fallback" values, and updates them
> individually when we read from the environment.

Nice problem description.  The fixed behavior makes sense to me, for
what it's worth.

Not about this patch, but: in case (1), shouldn't the author fall
back to $GIT_COMMITER_NAME?

^ permalink raw reply

* Re: [PATCHv2 3/8] ident: make user_ident_explicitly_given static
From: Jonathan Nieder @ 2012-11-15  7:51 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Felipe Contreras, Thomas Rast, Junio C Hamano
In-Reply-To: <20121115003405.GC17819@sigill.intra.peff.net>

Jeff King wrote:

> In v1.5.6-rc0~56^2 (2008-05-04) "user_ident_explicitly_given"
> was introduced as a global for communication between config,
> ident, and builtin-commit.  In v1.7.0-rc0~72^2 (2010-01-07)
> readers switched to using the common wrapper
> user_ident_sufficiently_given().  After v1.7.11-rc1~15^2~18
> (2012-05-21), the var is only written in ident.c.
>
> Now we can make it static, which will enable further
> refactoring without worrying about upsetting other code.
>
> Signed-off-by: Jeff King <peff@peff.net>

For what it's worth I liked the old commit message more.  But I don't
mind either.

Hope that helps,
Jonathan

^ permalink raw reply

* Re: [PATCHv2 2/8] t7502: factor out autoident prerequisite
From: Jonathan Nieder @ 2012-11-15  7:49 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Felipe Contreras, Thomas Rast, Junio C Hamano
In-Reply-To: <20121115003340.GB17819@sigill.intra.peff.net>

Jeff King wrote:

> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -738,6 +738,12 @@ test_lazy_prereq UTF8_NFD_TO_NFC '
>  	esac
>  '
>  
> +test_lazy_prereq AUTOIDENT '
> +	sane_unset GIT_AUTHOR_NAME &&
> +	sane_unset GIT_AUTHOR_EMAIL &&
> +	git var GIT_AUTHOR_IDENT
> +'

Lazy prereq scripts run in a subshell, so this should be safe.  Ack.

Thanks,
Jonathan

^ permalink raw reply

* Re: [PATCH v3 0/5] push: update remote tags only with force
From: Angelo Borsotti @ 2012-11-15  7:48 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Chris Rorvick, git, Drew Northup, Michael Haggerty, Philip Oakley,
	Johannes Sixt, Kacper Kornet, Jeff King, Felipe Contreras
In-Reply-To: <7v8va3afrt.fsf@alter.siamese.dyndns.org>

Hi Junio,

> I am *not* convinced that the "refs/tags/ is the only special
> hierarchy whose contents should not move" is a bad limitation we
> should avoid, but if it indeed is a bad limitation, the above is one
> possible way to think about avoiding it.

What other hierarchy besides branches and tags is there? Do you have
in mind some other that should not move?

-Angelo

On 15 November 2012 01:09, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> Addendum.
>
>> In any case, I thought this series was about users who run "push"
>> voluntarily stopping themselves from pushing updates to tags that
>> may happen to fast-forward, so if we were to go with the
>> configuration route, the suggestion would be more like
>>
>>     [push]
>>       updateNeedsForce = refs/tags/:refs/frotz/
>>
>> or perhaps
>>
>>     [remote "origin"]
>>       updateNeedsForce = refs/tags/:refs/frotz/
>>
>> if we want to configure it per-remote, to specify that you would
>> need to say "--force" to update the refs in the listed hierarchies.
>>
>> Then your patch series could become just the matter of declaring
>> that the value of push.updateNeedsForce, when unspecified, defaults
>> to "refs/tags/".
>
> The above is not a "you should do it this way" suggestion, by the
> way.
>
> I was just explaining what I meant by "it may be a good feature, but
> may not necessarily be limited to refs/tags" in my earlier message
> in a different way "... and a possible design that lifts the
> limitation may go like this".
>
> I am *not* convinced that the "refs/tags/ is the only special
> hierarchy whose contents should not move" is a bad limitation we
> should avoid, but if it indeed is a bad limitation, the above is one
> possible way to think about avoiding it.
>
> Thanks.

^ permalink raw reply

* Re: [PATCHv2 1/8] test-lib: allow negation of prerequisites
From: Jonathan Nieder @ 2012-11-15  7:46 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Felipe Contreras, Thomas Rast, Junio C Hamano
In-Reply-To: <20121115003325.GA17819@sigill.intra.peff.net>

Jeff King wrote:

> +test_expect_success !LAZY_TRUE 'missing lazy prereqs skip tests' '

I have a visceral nervousness when reading this code, from too much
unpleasant experience of bash's csh-style !history expansion.  Luckily
bash does not treat ! specially in the '-o sh' mode used by tests.

Does this feature work when running a test explicitly using
"bash <name of test>"?  That's something I do from time to time to
figure out whether a weird behavior is shell-specific.

If it works everywhere, this patch would help me conquer my fear of
exclamation points in git's tests, which would be a comfort to me and
a very good thing.

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Torsten Bögershausen @ 2012-11-15  5:54 UTC (permalink / raw)
  To: Jeff King; +Cc: Mark Levedahl, Torsten Bögershausen, Junio C Hamano, git
In-Reply-To: <20121115015608.GB19131@sigill.intra.peff.net>

On 15.11.12 02:56, Jeff King wrote:
> On Wed, Nov 14, 2012 at 08:50:43PM -0500, Mark Levedahl wrote:
> 
>> Cygwin changed the win32api implementation, and the old is not just
>> no longer supported for the current release series, but virtually
>> impossible to even install (several new packages are now installed,
>> the old package is in the "obsolete" category, i.e., not available).
>> The older cygwin 1.5 dll + utilities can be installed afresh, so that
>> is why I set up to switch based upon dll version - the proposed
>> test(s) and configuration would be to have git maintain compatibility
>> with an unsupported Cygwin configuration. I just don't think this is
>> worth the maintenance burden, but of course I am not the maintainer,
>> just expressing my opinion.
> 
> OK. I don't have a strong opinion either, as I don't know what's normal
> in the Cygwin world, and that is probably the most important thing to
> follow for the default. I got the impression that "normal" is changing
> to the new way, but Torsten's message made me wonder if were there quite
> yet (if there was some issue with upgrades versus new fresh installs).
> 
> But I have no real cygwin knowledge, so I'll bow out and let you guys
> discuss.
> 

My understanding:
Either use people cygwin 1.5 or they use cygwin 1.7, and in this case
the installation is updated frequently.

Peff or Junio, please go ahead with the patch.

If it turns out that we want to support cygwin installations like 1.7.7
which could be upgraded, but are not upgraded since they are
"production machines we do not dare to touch" we can still improve
the autodetection.

Thanks for the responses.
/Torsten

^ permalink raw reply

* [PATCH 1/5 v2] t/test-lib-functions.sh: allow to specify the tag name to test_commit
From: Brandon Casey @ 2012-11-15  5:49 UTC (permalink / raw)
  To: kraai; +Cc: git, Brandon Casey, Brandon Casey
In-Reply-To: <20121115032005.GA20677@ftbfs.org>

The <message> part of test_commit() may not be appropriate for a tag name.
So let's allow test_commit to accept a fourth argument to specify the tag
name.

Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 t/test-lib-functions.sh | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8889ba5..9e2b8b8 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -135,12 +135,13 @@ test_pause () {
 	fi
 }
 
-# Call test_commit with the arguments "<message> [<file> [<contents>]]"
+# Call test_commit with the arguments "<message> [<file> [<contents> [<tag>]]]"
 #
 # This will commit a file with the given contents and the given commit
-# message.  It will also add a tag with <message> as name.
+# message.  It will also add a tag with <message> as name unless <tag> is
+# given.
 #
-# Both <file> and <contents> default to <message>.
+# <file>, <contents>, and <tag> all default to <message>.
 
 test_commit () {
 	notick= &&
@@ -168,7 +169,7 @@ test_commit () {
 		test_tick
 	fi &&
 	git commit $signoff -m "$1" &&
-	git tag "$1"
+	git tag "${4:-$1}"
 }
 
 # Call test_merge with the arguments "<message> <commit>", where <commit>
-- 
1.8.0

^ permalink raw reply related

* Re: [PATCH 1/5] t/test-lib-functions.sh: allow to specify the tag name to test_commit
From: Brandon Casey @ 2012-11-15  5:43 UTC (permalink / raw)
  To: Matt Kraai; +Cc: git@vger.kernel.org, Brandon Casey
In-Reply-To: <20121115032005.GA20677@ftbfs.org>

Good eye.  Thanks.

On Wed, Nov 14, 2012 at 7:20 PM, Matt Kraai <kraai@ftbfs.org> wrote:
> On Wed, Nov 14, 2012 at 05:37:50PM -0800, Brandon Casey wrote:
>> -# Both <file> and <contents> default to <message>.
>> +# Both <file> <contents> and <tag> default to <message>.
>
> I think this line would be better as
>
>  # <file>, <contents>, and <tag> all default to <message>.
>
> since there's now more than two arguments that default to message.
>
> --
> Matt

^ permalink raw reply

* Re: Local clones aka forks disk size optimization
From: Sitaram Chamarty @ 2012-11-15  3:44 UTC (permalink / raw)
  To: Andrew Ardill; +Cc: Javier Domingo, git@vger.kernel.org
In-Reply-To: <CAH5451=Tk=zjkYbK0720VBkAA12VRCAE_Dx8bBkoXba60ho8AA@mail.gmail.com>

On Thu, Nov 15, 2012 at 7:04 AM, Andrew Ardill <andrew.ardill@gmail.com> wrote:
> On 15 November 2012 12:15, Javier Domingo <javierdo1@gmail.com> wrote:
>> Hi Andrew,
>>
>> Doing this would require I got tracked which one comes from which. So
>> it would imply some logic (and db) over it. With the hardlinking way,
>> it wouldn't require anything. The idea is that you don't have to do
>> anything else in the server.
>>
>> I understand that it would be imposible to do it for windows users
>> (but using cygwin), but for *nix ones yes...
>> Javier Domingo
>
> Paraphrasing from git-clone(1):
>
> When cloning a repository, if the source repository is specified with
> /path/to/repo syntax, the default is to clone the repository by making
> a copy of HEAD and everything under objects and refs directories. The
> files under .git/objects/ directory are hardlinked to save space when
> possible. To force copying instead of hardlinking (which may be
> desirable if you are trying to make a back-up of your repository)
> --no-hardlinks can be used.
>
> So hardlinks should be used where possible, and if they are not try
> upgrading Git.
>
> I think that covers all the use cases you have?

I am not sure it does.  My understanding is this:

'git clone -l' saves space on the initial clone, but subsequent pushes
end up with the same objects duplicated across all the "forks"
(assuming most of the forks keep up with some canonical repo).

The alternates mechanism can give you ongoing savings (as long as you
push to the "main" repo first), but it is dangerous, in the words of
the git-clone manpage.  You have to be confident no one will delete a
ref from the "main" repo and then do a gc or let it auto-gc.

He's looking for something that addresses both these issues.

As an additional idea, I suspect this is what the namespaces feature
was created for, but I am not sure, and have never played with it till
now.

Maybe someone who knows namespaces very well will chip in...

^ permalink raw reply

* Re: [PATCH 1/5] t/test-lib-functions.sh: allow to specify the tag name to test_commit
From: Matt Kraai @ 2012-11-15  3:20 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git, Brandon Casey
In-Reply-To: <1352943474-15573-1-git-send-email-drafnel@gmail.com>

On Wed, Nov 14, 2012 at 05:37:50PM -0800, Brandon Casey wrote:
> -# Both <file> and <contents> default to <message>.
> +# Both <file> <contents> and <tag> default to <message>.

I think this line would be better as

 # <file>, <contents>, and <tag> all default to <message>.

since there's now more than two arguments that default to message.

-- 
Matt

^ permalink raw reply

* Re: [PATCH] Add tcsh-completion support to contrib by using git-completion.bash
From: Felipe Contreras @ 2012-11-15  2:40 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Marc Khouzam, git
In-Reply-To: <20121114001159.GA14631@goldbirke>

On Wed, Nov 14, 2012 at 1:11 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> On Tue, Nov 13, 2012 at 07:31:45PM +0100, Felipe Contreras wrote:
>> On Mon, Nov 12, 2012 at 9:07 PM, Marc Khouzam <marc.khouzam@gmail.com> wrote:
>> > +       # Call _git() or _gitk() of the bash script, based on the first
>> > +       # element of the command-line
>> > +       _${COMP_WORDS[0]}
>>
>> You might want to use __${COMP_WORDS[0]}_main instead.
>
> That wouldn't work.  __git_main() doesn't set up the
> command-line-specific variables, but the wrapper around it does.

Yeah, but you can set those command-line-specific variables manually,
like the zsh completion wrapper does.

The problem with the _git wrapper is that it will call the
bash-specific complete command.

>> > +# Make the script executable if it is not
>> > +if ( ! -x ${__git_tcsh_completion_script} ) then
>> > +       chmod u+x ${__git_tcsh_completion_script}
>> > +endif
>>
>> Why not just source it?
>
> The goal is to re-use a Bash script to do completion in tcsh.  They
> are two different breeds, tcsh doesn't grok bash.  So sourcing the
> completion script is not an option, but we can still run it via Bash
> and use it's results.

I see, but the tcsh script can do something like this:

bash <<\EOF
echo $BASH
\EOF

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-15  2:14 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121115015052.GA19131@sigill.intra.peff.net>

On Thu, Nov 15, 2012 at 2:50 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Nov 15, 2012 at 02:41:50AM +0100, Felipe Contreras wrote:
>
>> But that I meant that when I introduce a regression it's like I'm
>> killing all that is good and sacred about git, and when you do it's
>> everything but that.
>
> The rhetoric in this statement is a good indication that there is
> nothing productive to come from our discussing it anymore.

The point is still true.

>> > If you want to seriously propose changing the behavior of "git commit",
>> > I think the best thing would be to make a real patch, laying out the
>> > pros and cons in the commit message, and post it. I would not be
>> > surprised if the other list participants have stopped reading our thread
>> > at this point, and the idea is going otherwise unnoticed.
>>
>> I would, if I saw any chance in it actually going through.
>
> Well, it certainly will not go through if you do not try.

At least I wouldn't be wasting my time.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
From: Felipe Contreras @ 2012-11-15  2:08 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121115003640.GH17819@sigill.intra.peff.net>

On Thu, Nov 15, 2012 at 1:36 AM, Jeff King <peff@peff.net> wrote:

> diff --git a/git-send-email.perl b/git-send-email.perl
> index 5a7c29d..0c49b32 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -436,9 +436,8 @@ if (0) {
>         }
>  }
>
> -my ($repoauthor, $repocommitter);
> -($repoauthor) = Git::ident_person(@repo, 'author');
> -($repocommitter) = Git::ident_person(@repo, 'committer');
> +my ($repoauthor, $author_explicit) = Git::ident_person(@repo, 'author');
> +my ($repocommitter, $committer_explicit) = Git::ident_person(@repo, 'committer');
>
>  # Verify the user input
>
> @@ -755,12 +754,17 @@ if (!$force) {
>
>  my $prompting = 0;
>  if (!defined $sender) {
> -       $sender = $repoauthor || $repocommitter || '';
> -       $sender = ask("Who should the emails appear to be from? [$sender] ",
> -                     default => $sender,
> -                     valid_re => qr/\@.*\./, confirm_only => 1);
> -       print "Emails will be sent from: ", $sender, "\n";
> -       $prompting++;
> +       ($sender, my $explicit) =
> +               defined $repoauthor ? ($repoauthor, $author_explicit) :
> +               defined $repocommitter ? ($repocommitter, $committer_explicit) :
> +               ('', 0);
> +       if (!$explicit) {
> +               $sender = ask("Who should the emails appear to be from? [$sender] ",
> +                             default => $sender,
> +                             valid_re => qr/\@.*\./, confirm_only => 1);
> +               print "Emails will be sent from: ", $sender, "\n";
> +               $prompting++;
> +       }
>  }
>
>  if (!@initial_to && !defined $to_cmd) {

I don't think there's any need for all that, this does the trick:

diff --git a/git-send-email.perl b/git-send-email.perl
index aea66a0..503e551 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -748,16 +748,11 @@ if (!$force) {
        }
 }

-my $prompting = 0;
 if (!defined $sender) {
        $sender = $repoauthor || $repocommitter || '';
-       $sender = ask("Who should the emails appear to be from? [$sender] ",
-                     default => $sender,
-                     valid_re => qr/\@.*\./, confirm_only => 1);
-       print "Emails will be sent from: ", $sender, "\n";
-       $prompting++;
 }

+my $prompting = 0;

This passes all the current tests and the ones you added.

Which kind of user will get the prompt with your patch, that would
miss it with mine?

Cheers.

-- 
Felipe Contreras

^ permalink raw reply related

* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Jeff King @ 2012-11-15  1:56 UTC (permalink / raw)
  To: Mark Levedahl; +Cc: Torsten Bögershausen, Junio C Hamano, git
In-Reply-To: <50A44A73.8020007@gmail.com>

On Wed, Nov 14, 2012 at 08:50:43PM -0500, Mark Levedahl wrote:

> Cygwin changed the win32api implementation, and the old is not just
> no longer supported for the current release series, but virtually
> impossible to even install (several new packages are now installed,
> the old package is in the "obsolete" category, i.e., not available).
> The older cygwin 1.5 dll + utilities can be installed afresh, so that
> is why I set up to switch based upon dll version - the proposed
> test(s) and configuration would be to have git maintain compatibility
> with an unsupported Cygwin configuration. I just don't think this is
> worth the maintenance burden, but of course I am not the maintainer,
> just expressing my opinion.

OK. I don't have a strong opinion either, as I don't know what's normal
in the Cygwin world, and that is probably the most important thing to
follow for the default. I got the impression that "normal" is changing
to the new way, but Torsten's message made me wonder if were there quite
yet (if there was some issue with upgrades versus new fresh installs).

But I have no real cygwin knowledge, so I'll bow out and let you guys
discuss.

-Peff

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Mark Levedahl @ 2012-11-15  1:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Torsten Bögershausen, Junio C Hamano, git
In-Reply-To: <20121115001635.GA17370@sigill.intra.peff.net>

On 11/14/2012 07:16 PM, Jeff King wrote:
> On Wed, Nov 14, 2012 at 10:13:28PM +0100, Torsten Bögershausen wrote:
> b) Autodetection:
>    (Just loud thinking), running
> $grep mingw /usr/include/w32api/winsock2.h
>   * This file is part of the mingw-w64 runtime package.
> #include <_mingw_unicode.h>
>
> on cygwin 1.7.17 indicates that we can use grep in the Makefile to
> autodetect the "mingw headers"
> Hmm. Can we rely on the /usr/include bit, though?
>
> I assume a test-compile would be sufficient, but currently we do not do
> anything more magic than "uname" in the Makefile itself to determine
> defaults.  Maybe it would be better to do the detection in the configure
> script? And then eventually flip the default in the Makefile once
> sufficient time has passed for most people to want the new format (which
> would not be necessary for people using autoconf, but would help people
> who do not).
>
> -Peff
>

Cygwin changed the win32api implementation, and the old is not just no 
longer supported for the current release series, but virtually 
impossible to even install (several new packages are now installed, the 
old package is in the "obsolete" category, i.e., not available). The 
older cygwin 1.5 dll + utilities can be installed afresh, so that is why 
I set up to switch based upon dll version - the proposed test(s) and 
configuration would be to have git maintain compatibility with an 
unsupported Cygwin configuration. I just don't think this is worth the 
maintenance burden, but of course I am not the maintainer, just 
expressing my opinion.

I have no trouble renaming the macro to whatever seems to clarify things.

Mark

^ permalink raw reply

* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-15  1:50 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <CAMP44s3vYP6zR6sKxgVSoZUySxcAuR7MioMeT1WDAnU0PtZKGA@mail.gmail.com>

On Thu, Nov 15, 2012 at 02:41:50AM +0100, Felipe Contreras wrote:

> But that I meant that when I introduce a regression it's like I'm
> killing all that is good and sacred about git, and when you do it's
> everything but that.

The rhetoric in this statement is a good indication that there is
nothing productive to come from our discussing it anymore.

> > If you want to seriously propose changing the behavior of "git commit",
> > I think the best thing would be to make a real patch, laying out the
> > pros and cons in the commit message, and post it. I would not be
> > surprised if the other list participants have stopped reading our thread
> > at this point, and the idea is going otherwise unnoticed.
> 
> I would, if I saw any chance in it actually going through.

Well, it certainly will not go through if you do not try.

-Peff

^ permalink raw reply

* [PATCH] usage.c: detect recursion in die routines and bail out immediately
From: Brandon Casey @ 2012-11-15  1:45 UTC (permalink / raw)
  To: gitster; +Cc: git, Brandon Casey, Brandon Casey

It is theoretically possible for a die handler to get into a state of
infinite recursion.  For example, if a die handler called another function
which itself called die().  Let's at least detect this situation, inform the
user, and call exit.

Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
 usage.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/usage.c b/usage.c
index a2a6678..8eab281 100644
--- a/usage.c
+++ b/usage.c
@@ -6,6 +6,8 @@
 #include "git-compat-util.h"
 #include "cache.h"
 
+static int dying;
+
 void vreportf(const char *prefix, const char *err, va_list params)
 {
 	char msg[4096];
@@ -82,6 +84,12 @@ void NORETURN die(const char *err, ...)
 {
 	va_list params;
 
+	if (dying) {
+		fputs("fatal: recursion detected in die handler\n", stderr);
+		exit(128);
+	}
+	dying = 1;
+
 	va_start(params, err);
 	die_routine(err, params);
 	va_end(params);
@@ -94,6 +102,13 @@ void NORETURN die_errno(const char *fmt, ...)
 	char str_error[256], *err;
 	int i, j;
 
+	if (dying) {
+		fputs("fatal: recursion detected in die_errno handler\n",
+			stderr);
+		exit(128);
+	}
+	dying = 1;
+
 	err = strerror(errno);
 	for (i = j = 0; err[i] && j < sizeof(str_error) - 1; ) {
 		if ((str_error[j++] = err[i++]) != '%')
-- 
1.8.0

^ 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