Re: [PATCH 1/3] introduce GIT_INDEX_VERSION environment variable

2014-02-21 Thread Thomas Gummerer 
Junio C Hamano gits...@pobox.com writes:

 Thomas Gummerer t.gumme...@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 000..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 21 | 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] introduce GIT_INDEX_VERSION environment variable

2014-02-18 Thread Junio C Hamano 
Thomas Gummerer t.gumme...@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.

 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.

 diff --git a/t/t1600-index.sh b/t/t1600-index.sh
 new file mode 100755
 index 000..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 21 | 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?

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

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] introduce GIT_INDEX_VERSION environment variable

2014-02-15 Thread Thomas Gummerer 
Respect a GIT_INDEX_VERSION environment variable, when a new index is
initialized.  Setting the environment variable will not cause existing
index files to be converted to another format, but will only affect
newly written index files.  This can be used to initialize repositories
with index-v4.

Signed-off-by: Thomas Gummerer t.gumme...@gmail.com
---
 Documentation/git.txt |  5 +
 read-cache.c  | 18 +-
 t/t1600-index.sh  | 24 
 3 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100755 t/t1600-index.sh

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.
+
 'GIT_OBJECT_DIRECTORY'::
If the object storage directory is specified via this
environment variable then the sha1 directories are created
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 (version  INDEX_FORMAT_LB || version  INDEX_FORMAT_UB) {
+   warning(_(GIT_INDEX_VERSION set, but the value is 
invalid.\n
+ Using version %i), INDEX_FORMAT_DEFAULT);
+   version = INDEX_FORMAT_DEFAULT;
+   }
+   return version;
+   }
+}
+
 /*
  * dev/ino/uid/gid/size are also just tracked to the low 32 bits
  * Again - this is just a (very strong in practice) heuristic that
@@ -1799,7 +1815,7 @@ int write_index(struct index_state *istate, int newfd)
}
 
if (!istate-version)
-   istate-version = INDEX_FORMAT_DEFAULT;
+   istate-version = get_index_format_default();
 
/* demote version 3 to version 2 when the latter suffices */
if (istate-version == 3 || istate-version == 2)
diff --git a/t/t1600-index.sh b/t/t1600-index.sh
new file mode 100755
index 000..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 21 | 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
+   )
+'
+
+test_done
-- 
1.8.5.2.300.ge613be6.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html