public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Thomas Gummerer <t.gummerer@gmail•com>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org, Jonathan Nieder <jrnieder@gmail•com>
Subject: Re: [PATCH 1/3] introduce GIT_INDEX_VERSION environment variable
Date: Fri, 21 Feb 2014 23:02:53 +0100	[thread overview]
Message-ID: <87d2ig5dki.fsf@hank.lan> (raw)
In-Reply-To: <xmqqsirg9c4z.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox•com> writes:

> Thomas Gummerer <t.gummerer@gmail•com> writes:
>
>> diff --git a/Documentation/git.txt b/Documentation/git.txt
>> index aec3726..bc9eeea 100644
>> --- a/Documentation/git.txt
>> +++ b/Documentation/git.txt
>> @@ -712,6 +712,11 @@ Git so take care if using Cogito etc.
>>  	index file. If not specified, the default of `$GIT_DIR/index`
>>  	is used.
>>  
>> +'GIT_INDEX_VERSION'::
>> +	This environment variable allows the specification of an index
>> +	version for new repositories.  It won't affect existing index
>> +	files.  By default index file version 3 is used.
>> +
>
> This is half-correct, isn't it?  In-code variable may say version 3
> but we demote it to version 2 unless we absolutely need to use the
> version 3 ugliness.

Yes, you're right, to be correct we should say [23] instead of 3 here
maybe?

>> diff --git a/read-cache.c b/read-cache.c
>> index 3f735f3..3993e12 100644
>> --- a/read-cache.c
>> +++ b/read-cache.c
>> @@ -1223,6 +1223,22 @@ static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int reall
>>  
>>  #define INDEX_FORMAT_DEFAULT 3
>>  
>> +static unsigned int get_index_format_default()
>> +{
>> +	char *envversion = getenv("GIT_INDEX_VERSION");
>> +	if (!envversion) {
>> +		return INDEX_FORMAT_DEFAULT;
>> +	} else {
>> +		unsigned int version = strtol(envversion, NULL, 10);
>
> If we are using strtol() to parse it carefully, we should make sure
> it parses to the end by giving a non-NULL second argument and
> checking where the parsing stopped.

Thanks, makes sense, will fix it in the re-roll.

>> diff --git a/t/t1600-index.sh b/t/t1600-index.sh
>> new file mode 100755
>> index 0000000..37fd84d
>> --- /dev/null
>> +++ b/t/t1600-index.sh
>> @@ -0,0 +1,24 @@
>> +#!/bin/sh
>> +
>> +test_description='index file specific tests'
>> +
>> +. ./test-lib.sh
>> +
>> +test_expect_success 'setup' '
>> +	echo 1 >a
>> +'
>> +
>> +test_expect_success 'out of bounds GIT_INDEX_VERSION issues warning' '
>> +	(
>> +		GIT_INDEX_VERSION=1 &&
>> +		export GIT_INDEX_VERSION &&
>> +		git add a 2>&1 | sed "s/[0-9]//" >actual.err &&
>> +		sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
>> +			warning: GIT_INDEX_VERSION set, but the value is invalid.
>> +			Using version Z
>> +		EOF
>> +		test_i18ncmp expect.err actual.err
>> +	)
>> +'
>
> If we already have an index in version N when this test starts, what
> should happen?

I think we shouldn't print anything, since we won't change the file
format.  I'll add another test for that.

> Will queue on 'pu', with some suggested fixups.

Thanks, I think both fixups in 'pu' make sense, so I'll include them in
the re-roll.

> Thanks.

next prev parent reply	other threads:[~2014-02-21 22:02 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-15 19:23 [PATCH 0/3] Wider exposure for index-v4 Thomas Gummerer
2014-02-15 19:23 ` [PATCH 1/3] introduce GIT_INDEX_VERSION environment variable Thomas Gummerer
2014-02-19  0:31   ` Junio C Hamano
2014-02-21 22:02     ` Thomas Gummerer [this message]
2014-02-15 19:23 ` [PATCH 2/3] test-lib: allow setting the index format version Thomas Gummerer
2014-02-19  0:31   ` Junio C Hamano
2014-02-15 19:23 ` [PATCH 3/3] read-cache: add index.version config variable Thomas Gummerer
2014-02-16  2:10   ` Eric Sunshine
2014-02-16  1:16 ` [PATCH 0/3] Wider exposure for index-v4 Duy Nguyen
2014-02-16 10:37   ` Thomas Gummerer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d2ig5dki.fsf@hank.lan \
    --to=t.gummerer@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=jrnieder@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

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