* [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support @ 2018-09-12 21:26 Ben Peart 2018-09-12 22:31 ` Thomas Gummerer ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Ben Peart @ 2018-09-12 21:26 UTC (permalink / raw) To: git@vger.kernel.org; +Cc: gitster@pobox.com, Ben Peart, Ben Peart Teach get_index_format_default() to support running the test suite with specific index versions. In particular, this enables the test suite to be run using index version 4 which is not the default so gets less testing. Signed-off-by: Ben Peart <Ben.Peart@microsoft.com> --- Notes: Base Ref: v2.19.0 Web-Diff: https://github.com/benpeart/git/commit/52e733e2ce Checkout: git fetch https://github.com/benpeart/git git-test-index-version-v1 && git checkout 52e733e2ce read-cache.c | 47 +++++++++++++++++++++++++++++++++-------------- t/README | 6 +++++- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/read-cache.c b/read-cache.c index 7b1354d759..d140ce9989 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1570,26 +1570,45 @@ static unsigned int get_index_format_default(void) char *envversion = getenv("GIT_INDEX_VERSION"); char *endp; int value; - unsigned int version = INDEX_FORMAT_DEFAULT; + unsigned int version = -1; + + if (envversion) { + version = strtoul(envversion, &endp, 10); + if (*endp || + version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { + warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n" + "Using version %i"), INDEX_FORMAT_DEFAULT); + version = INDEX_FORMAT_DEFAULT; + } + } - if (!envversion) { - if (!git_config_get_int("index.version", &value)) + if (version == -1) { + if (!git_config_get_int("index.version", &value)) { version = value; - if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { - warning(_("index.version set, but the value is invalid.\n" - "Using version %i"), INDEX_FORMAT_DEFAULT); - return INDEX_FORMAT_DEFAULT; + if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { + warning(_("index.version set, but the value is invalid.\n" + "Using version %i"), INDEX_FORMAT_DEFAULT); + version = INDEX_FORMAT_DEFAULT; + } } - return version; } - version = strtoul(envversion, &endp, 10); - if (*endp || - version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { - warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n" - "Using version %i"), INDEX_FORMAT_DEFAULT); - version = INDEX_FORMAT_DEFAULT; + if (version == -1) { + envversion = getenv("GIT_TEST_INDEX_VERSION"); + if (envversion) { + version = strtoul(envversion, &endp, 10); + if (*endp || + version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { + warning(_("GIT_TEST_INDEX_VERSION set, but the value is invalid.\n" + "Using version %i"), INDEX_FORMAT_DEFAULT); + version = INDEX_FORMAT_DEFAULT; + } + } } + + if (version == -1) + version = INDEX_FORMAT_DEFAULT; + return version; } diff --git a/t/README b/t/README index 9028b47d92..f872638a78 100644 --- a/t/README +++ b/t/README @@ -315,10 +315,14 @@ packs on demand. This normally only happens when the object size is over 2GB. This variable forces the code path on any object larger than <n> bytes. -GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon pack-objects code +GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncommon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_INDEX_VERSION=<n> exercises the index read/write code path +for the index version specified. Can be set to any valid version +but the non-default version 4 is probably the most beneficial. + Naming Tests ------------ base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670 -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support 2018-09-12 21:26 [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support Ben Peart @ 2018-09-12 22:31 ` Thomas Gummerer 2018-09-13 14:07 ` Ben Peart 2018-09-13 18:17 ` [PATCH v2] read-cache: update TEST_GIT_INDEX_VERSION support Ben Peart 2018-09-13 18:49 ` [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support Ævar Arnfjörð Bjarmason 2 siblings, 1 reply; 10+ messages in thread From: Thomas Gummerer @ 2018-09-12 22:31 UTC (permalink / raw) To: Ben Peart; +Cc: git@vger.kernel.org, gitster@pobox.com, Ben Peart On 09/12, Ben Peart wrote: > Teach get_index_format_default() to support running the test suite > with specific index versions. In particular, this enables the test suite > to be run using index version 4 which is not the default so gets less testing. I found this commit message slightly misleading. Running the test suite with specific index versions is already supported, by defining TEST_GIT_INDEX_VERSION in 'config.mak'. What we're doing here is introduce an additional environment variable that can also be used to set the index format in tests. Even setting TEST_GIT_INDEX_VERSION=4 in the environment does run the test suite with index-v4. Admittedly the name is a bit strange compared to our usual GIT_TEST_* environment variable names, and it should probably be documented better (it's only documented in the Makefile currently), but I'm not sure we should introduce another environment variable for this purpose? > Signed-off-by: Ben Peart <Ben.Peart@microsoft.com> > --- > > Notes: > Base Ref: v2.19.0 > Web-Diff: https://github.com/benpeart/git/commit/52e733e2ce > Checkout: git fetch https://github.com/benpeart/git git-test-index-version-v1 && git checkout 52e733e2ce > > read-cache.c | 47 +++++++++++++++++++++++++++++++++-------------- > t/README | 6 +++++- > 2 files changed, 38 insertions(+), 15 deletions(-) > > diff --git a/read-cache.c b/read-cache.c > index 7b1354d759..d140ce9989 100644 > --- a/read-cache.c > +++ b/read-cache.c > @@ -1570,26 +1570,45 @@ static unsigned int get_index_format_default(void) > char *envversion = getenv("GIT_INDEX_VERSION"); > char *endp; > int value; > - unsigned int version = INDEX_FORMAT_DEFAULT; > + unsigned int version = -1; > + > + if (envversion) { > + version = strtoul(envversion, &endp, 10); > + if (*endp || > + version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { > + warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n" > + "Using version %i"), INDEX_FORMAT_DEFAULT); > + version = INDEX_FORMAT_DEFAULT; > + } > + } > > - if (!envversion) { > - if (!git_config_get_int("index.version", &value)) > + if (version == -1) { > + if (!git_config_get_int("index.version", &value)) { > version = value; > - if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { > - warning(_("index.version set, but the value is invalid.\n" > - "Using version %i"), INDEX_FORMAT_DEFAULT); > - return INDEX_FORMAT_DEFAULT; > + if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { > + warning(_("index.version set, but the value is invalid.\n" > + "Using version %i"), INDEX_FORMAT_DEFAULT); > + version = INDEX_FORMAT_DEFAULT; > + } > } > - return version; > } > > - version = strtoul(envversion, &endp, 10); > - if (*endp || > - version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { > - warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n" > - "Using version %i"), INDEX_FORMAT_DEFAULT); > - version = INDEX_FORMAT_DEFAULT; > + if (version == -1) { > + envversion = getenv("GIT_TEST_INDEX_VERSION"); > + if (envversion) { > + version = strtoul(envversion, &endp, 10); > + if (*endp || > + version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { > + warning(_("GIT_TEST_INDEX_VERSION set, but the value is invalid.\n" > + "Using version %i"), INDEX_FORMAT_DEFAULT); > + version = INDEX_FORMAT_DEFAULT; > + } > + } > } > + > + if (version == -1) > + version = INDEX_FORMAT_DEFAULT; > + > return version; > } > > diff --git a/t/README b/t/README > index 9028b47d92..f872638a78 100644 > --- a/t/README > +++ b/t/README > @@ -315,10 +315,14 @@ packs on demand. This normally only happens when the object size is > over 2GB. This variable forces the code path on any object larger than > <n> bytes. > > -GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon pack-objects code > +GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncommon pack-objects code > path where deltas larger than this limit require extra memory > allocation for bookkeeping. > > +GIT_TEST_INDEX_VERSION=<n> exercises the index read/write code path > +for the index version specified. Can be set to any valid version > +but the non-default version 4 is probably the most beneficial. > + > Naming Tests > ------------ > > > base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670 > -- > 2.18.0.windows.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support 2018-09-12 22:31 ` Thomas Gummerer @ 2018-09-13 14:07 ` Ben Peart 2018-09-13 21:56 ` Thomas Gummerer 0 siblings, 1 reply; 10+ messages in thread From: Ben Peart @ 2018-09-13 14:07 UTC (permalink / raw) To: Thomas Gummerer, Ben Peart Cc: git@vger.kernel.org, gitster@pobox.com, Ben Peart On 9/12/2018 6:31 PM, Thomas Gummerer wrote: > On 09/12, Ben Peart wrote: >> Teach get_index_format_default() to support running the test suite >> with specific index versions. In particular, this enables the test suite >> to be run using index version 4 which is not the default so gets less testing. > > I found this commit message slightly misleading. Running the test > suite with specific index versions is already supported, by defining > TEST_GIT_INDEX_VERSION in 'config.mak'. What we're doing here is > introduce an additional environment variable that can also be used to > set the index format in tests. > > Even setting TEST_GIT_INDEX_VERSION=4 in the environment does run the > test suite with index-v4. Admittedly the name is a bit strange > compared to our usual GIT_TEST_* environment variable names, and it > should probably be documented better (it's only documented in the > Makefile currently), but I'm not sure we should introduce another > environment variable for this purpose? TEST_GIT_INDEX_VERSION enables the testing I was looking for but you're right, it isn't well documented and the atypical naming and implementation don't help either. I checked the documentation and code but didn't see any way to test the V4 index code path so wrote this patch. I wonder if we can improve the discoverability of TEST_GIT_INDEX_VERSION through better naming and documentation. How about this as an alternative? diff --git a/Makefile b/Makefile index 5a969f5830..9e84ef02f7 100644 --- a/Makefile +++ b/Makefile @@ -400,7 +400,7 @@ all:: # (defaults to "man") if you want to have a different default when # "git help" is called without a parameter specifying the format. # -# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite +# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite # with a different indexfile format version. If it isn't set the index # file format used is index-v[23]. # @@ -2599,8 +2599,8 @@ endif ifdef GIT_INTEROP_MAKE_OPTS @echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+ endif -ifdef TEST_GIT_INDEX_VERSION - @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+ +ifdef GIT_TEST_INDEX_VERSION + @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+ endif @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi diff --git a/t/test-lib.sh b/t/test-lib.sh index 44288cbb59..31698c01c4 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -134,9 +134,9 @@ export EDITOR GIT_TRACE_BARE=1 export GIT_TRACE_BARE -if test -n "${TEST_GIT_INDEX_VERSION:+isset}" +if test -n "${GIT_TEST_INDEX_VERSION:+isset}" then - GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION" + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" export GIT_INDEX_VERSION fi diff --git a/t/README b/t/README index 9028b47d92..f872638a78 100644 --- a/t/README +++ b/t/README @@ -315,10 +315,14 @@ packs on demand. This normally only happens when the object size is over 2GB. This variable forces the code path on any object larger than <n> bytes. -GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon pack-objects code +GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncommon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_INDEX_VERSION=<n> exercises the index read/write code path +for the index version specified. Can be set to any valid version +but the non-default version 4 is probably the most beneficial. + Naming Tests ------------ ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support 2018-09-13 14:07 ` Ben Peart @ 2018-09-13 21:56 ` Thomas Gummerer 2018-09-13 22:08 ` Junio C Hamano 0 siblings, 1 reply; 10+ messages in thread From: Thomas Gummerer @ 2018-09-13 21:56 UTC (permalink / raw) To: Ben Peart; +Cc: Ben Peart, git@vger.kernel.org, gitster@pobox.com, Ben Peart On 09/13, Ben Peart wrote: > > > On 9/12/2018 6:31 PM, Thomas Gummerer wrote: > > On 09/12, Ben Peart wrote: > > > Teach get_index_format_default() to support running the test suite > > > with specific index versions. In particular, this enables the test suite > > > to be run using index version 4 which is not the default so gets less testing. > > > > I found this commit message slightly misleading. Running the test > > suite with specific index versions is already supported, by defining > > TEST_GIT_INDEX_VERSION in 'config.mak'. What we're doing here is > > introduce an additional environment variable that can also be used to > > set the index format in tests. > > > > Even setting TEST_GIT_INDEX_VERSION=4 in the environment does run the > > test suite with index-v4. Admittedly the name is a bit strange > > compared to our usual GIT_TEST_* environment variable names, and it > > should probably be documented better (it's only documented in the > > Makefile currently), but I'm not sure we should introduce another > > environment variable for this purpose? > > TEST_GIT_INDEX_VERSION enables the testing I was looking for but you're > right, it isn't well documented and the atypical naming and implementation > don't help either. > > I checked the documentation and code but didn't see any way to test the V4 > index code path so wrote this patch. I wonder if we can improve the > discoverability of TEST_GIT_INDEX_VERSION through better naming and > documentation. > > How about this as an alternative? Thanks, I do think this is a good idea. I do however share Ævar's concern in https://public-inbox.org/git/87h8itkz2h.fsf@evledraar.gmail.com/. I have TEST_GIT_INDEX_VERSION=4 set in my config.mak since quite a long time, and had I missed this thread, I would all of a sudden not run the test suite with index format 4 anymore without any notice. I think the suggestion of erroring out if TEST_GIT_INDEX_VERSION is set would be useful in this case (and probably others in which you're renaming these variables). Not sure how many people it would affect (and most of those would probably read the mailing list), but it's not a big change either. Btw, I think it would be nice to have all these renaming/documenting variables for the test suite patches in one series, so they are easier to look at with more context. > > diff --git a/Makefile b/Makefile > index 5a969f5830..9e84ef02f7 100644 > --- a/Makefile > +++ b/Makefile > @@ -400,7 +400,7 @@ all:: > # (defaults to "man") if you want to have a different default when > # "git help" is called without a parameter specifying the format. > # > -# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite > +# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite > # with a different indexfile format version. If it isn't set the index > # file format used is index-v[23]. > # > @@ -2599,8 +2599,8 @@ endif > ifdef GIT_INTEROP_MAKE_OPTS > @echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst > ','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+ > endif > -ifdef TEST_GIT_INDEX_VERSION > - @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst > ','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+ > +ifdef GIT_TEST_INDEX_VERSION > + @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst > ','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+ > > endif > @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 44288cbb59..31698c01c4 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -134,9 +134,9 @@ export EDITOR > GIT_TRACE_BARE=1 > export GIT_TRACE_BARE > > -if test -n "${TEST_GIT_INDEX_VERSION:+isset}" > +if test -n "${GIT_TEST_INDEX_VERSION:+isset}" > then > - GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION" > + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" > export GIT_INDEX_VERSION > fi > > diff --git a/t/README b/t/README > index 9028b47d92..f872638a78 100644 > --- a/t/README > +++ b/t/README > @@ -315,10 +315,14 @@ packs on demand. This normally only happens when the > object size is > over 2GB. This variable forces the code path on any object larger than > <n> bytes. > > -GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon pack-objects code > +GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncommon pack-objects code > path where deltas larger than this limit require extra memory > allocation for bookkeeping. > > +GIT_TEST_INDEX_VERSION=<n> exercises the index read/write code path > +for the index version specified. Can be set to any valid version > +but the non-default version 4 is probably the most beneficial. > + > Naming Tests > ------------ > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support 2018-09-13 21:56 ` Thomas Gummerer @ 2018-09-13 22:08 ` Junio C Hamano 2018-09-14 13:50 ` Ben Peart 0 siblings, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2018-09-13 22:08 UTC (permalink / raw) To: Thomas Gummerer; +Cc: Ben Peart, Ben Peart, git@vger.kernel.org, Ben Peart Thomas Gummerer <t.gummerer@gmail.com> writes: > Thanks, I do think this is a good idea. I do however share Ævar's > concern in https://public-inbox.org/git/87h8itkz2h.fsf@evledraar.gmail.com/. > I have TEST_GIT_INDEX_VERSION=4 set in my config.mak since quite a > long time, and had I missed this thread, I would all of a sudden not > run the test suite with index format 4 anymore without any notice. > > I think the suggestion of erroring out if TEST_GIT_INDEX_VERSION is > set would be useful in this case (and probably others in which you're > renaming these variables). I am not enthused by "you are using an old variable---we fail your build/test". The assumption is that people let config.mak laying around regardless of how new/old the vintage of Git they are building and testing. I do not think you'd want to adjust your config.mak as you switch between 'maint' and 'next. I think it is OK to make it error only if the old one is set without the new one. Then people can have _both_ set to the same value during the period in which the topic sails through pu down to next down to master, after seeing an failure once while building and testing 'pu'. > Btw, I think it would be nice to have all these renaming/documenting > variables for the test suite patches in one series, so they are easier > to look at with more context. Yeah, even though these three were posted as independent changes, their additions to t/README inevitably conflicted with each other. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support 2018-09-13 22:08 ` Junio C Hamano @ 2018-09-14 13:50 ` Ben Peart 2018-09-14 16:20 ` Junio C Hamano 0 siblings, 1 reply; 10+ messages in thread From: Ben Peart @ 2018-09-14 13:50 UTC (permalink / raw) To: Junio C Hamano, Thomas Gummerer; +Cc: Ben Peart, git@vger.kernel.org, Ben Peart On 9/13/2018 6:08 PM, Junio C Hamano wrote: > Thomas Gummerer <t.gummerer@gmail.com> writes: > >> Thanks, I do think this is a good idea. I do however share Ævar's >> concern in https://public-inbox.org/git/87h8itkz2h.fsf@evledraar.gmail.com/. >> I have TEST_GIT_INDEX_VERSION=4 set in my config.mak since quite a >> long time, and had I missed this thread, I would all of a sudden not >> run the test suite with index format 4 anymore without any notice. >> >> I think the suggestion of erroring out if TEST_GIT_INDEX_VERSION is >> set would be useful in this case (and probably others in which you're >> renaming these variables). > > I am not enthused by "you are using an old variable---we fail your > build/test". The assumption is that people let config.mak laying > around regardless of how new/old the vintage of Git they are > building and testing. I do not think you'd want to adjust your > config.mak as you switch between 'maint' and 'next. > > I think it is OK to make it error only if the old one is set without > the new one. Then people can have _both_ set to the same value > during the period in which the topic sails through pu down to next > down to master, after seeing an failure once while building and > testing 'pu'. > I'll make it a warning if they are both set so that people are reminded to remove the old variable and an error if only the old one is set so people know to update their environment. >> Btw, I think it would be nice to have all these renaming/documenting >> variables for the test suite patches in one series, so they are easier >> to look at with more context. > > Yeah, even though these three were posted as independent changes, > their additions to t/README inevitably conflicted with each other. > > I'll combine all these into a single patch series. It grew as I discovered more that needed to be updated. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support 2018-09-14 13:50 ` Ben Peart @ 2018-09-14 16:20 ` Junio C Hamano 0 siblings, 0 replies; 10+ messages in thread From: Junio C Hamano @ 2018-09-14 16:20 UTC (permalink / raw) To: Ben Peart; +Cc: Thomas Gummerer, Ben Peart, git@vger.kernel.org, Ben Peart Ben Peart <peartben@gmail.com> writes: >> I think it is OK to make it error only if the old one is set without >> the new one. Then people can have _both_ set to the same value >> during the period in which the topic sails through pu down to next >> down to master, after seeing an failure once while building and >> testing 'pu'. >> > > I'll make it a warning if they are both set so that people are > reminded to remove the old variable and an error if only the old one > is set so people know to update their environment. After the topic graduates to a stable release (or two), it would be a good addition to encourage people to clean things up, of course, but doing so too early by warning when they are both set is not a good idea, I would think. It would bring us back to "the user has to futz with config.mak every time switching between 'maint' and 'next'". > I'll combine all these into a single patch series. It grew as I > discovered more that needed to be updated. Thanks. I didn't mind it too much to have them as independent patches. It at least helped me forcing myself to give closer look at them ;-) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] read-cache: update TEST_GIT_INDEX_VERSION support 2018-09-12 21:26 [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support Ben Peart 2018-09-12 22:31 ` Thomas Gummerer @ 2018-09-13 18:17 ` Ben Peart 2018-09-13 18:49 ` [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support Ævar Arnfjörð Bjarmason 2 siblings, 0 replies; 10+ messages in thread From: Ben Peart @ 2018-09-13 18:17 UTC (permalink / raw) To: Ben Peart; +Cc: Ben Peart, git@vger.kernel.org, gitster@pobox.com Rename TEST_GIT_INDEX_VERSION to GIT_TEST_INDEX_VERSION for consistency with the other GIT_TEST_ special setups and properly document its use. Signed-off-by: Ben Peart <Ben.Peart@microsoft.com> --- Notes: Base Ref: v2.19.0 Web-Diff: https://github.com/benpeart/git/commit/e26ccb9004 Checkout: git fetch https://github.com/benpeart/git git-test-index-version-v2 && git checkout e26ccb9004 ### Interdiff (v1..v2): diff --git a/Makefile b/Makefile index 5a969f5830..9e84ef02f7 100644 --- a/Makefile +++ b/Makefile @@ -400,7 +400,7 @@ all:: # (defaults to "man") if you want to have a different default when # "git help" is called without a parameter specifying the format. # -# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite +# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite # with a different indexfile format version. If it isn't set the index # file format used is index-v[23]. # @@ -2599,8 +2599,8 @@ endif ifdef GIT_INTEROP_MAKE_OPTS @echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+ endif -ifdef TEST_GIT_INDEX_VERSION - @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+ +ifdef GIT_TEST_INDEX_VERSION + @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+ endif @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi diff --git a/read-cache.c b/read-cache.c index d140ce9989..7b1354d759 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1570,45 +1570,26 @@ static unsigned int get_index_format_default(void) char *envversion = getenv("GIT_INDEX_VERSION"); char *endp; int value; - unsigned int version = -1; + unsigned int version = INDEX_FORMAT_DEFAULT; - if (envversion) { - version = strtoul(envversion, &endp, 10); - if (*endp || - version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { - warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n" - "Using version %i"), INDEX_FORMAT_DEFAULT); - version = INDEX_FORMAT_DEFAULT; - } - } - - if (version == -1) { - if (!git_config_get_int("index.version", &value)) { + if (!envversion) { + if (!git_config_get_int("index.version", &value)) version = value; if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { warning(_("index.version set, but the value is invalid.\n" "Using version %i"), INDEX_FORMAT_DEFAULT); - version = INDEX_FORMAT_DEFAULT; - } + return INDEX_FORMAT_DEFAULT; } + return version; } - if (version == -1) { - envversion = getenv("GIT_TEST_INDEX_VERSION"); - if (envversion) { version = strtoul(envversion, &endp, 10); if (*endp || version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { - warning(_("GIT_TEST_INDEX_VERSION set, but the value is invalid.\n" + warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n" "Using version %i"), INDEX_FORMAT_DEFAULT); version = INDEX_FORMAT_DEFAULT; } - } - } - - if (version == -1) - version = INDEX_FORMAT_DEFAULT; - return version; } diff --git a/t/README b/t/README index f872638a78..fb6359b17b 100644 --- a/t/README +++ b/t/README @@ -320,8 +320,7 @@ path where deltas larger than this limit require extra memory allocation for bookkeeping. GIT_TEST_INDEX_VERSION=<n> exercises the index read/write code path -for the index version specified. Can be set to any valid version -but the non-default version 4 is probably the most beneficial. +for the index version specified. Can be set to any valid version. Naming Tests ------------ diff --git a/t/test-lib.sh b/t/test-lib.sh index 44288cbb59..31698c01c4 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -134,9 +134,9 @@ export EDITOR GIT_TRACE_BARE=1 export GIT_TRACE_BARE -if test -n "${TEST_GIT_INDEX_VERSION:+isset}" +if test -n "${GIT_TEST_INDEX_VERSION:+isset}" then - GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION" + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" export GIT_INDEX_VERSION fi ### Patches Makefile | 6 +++--- t/README | 5 ++++- t/test-lib.sh | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 5a969f5830..9e84ef02f7 100644 --- a/Makefile +++ b/Makefile @@ -400,7 +400,7 @@ all:: # (defaults to "man") if you want to have a different default when # "git help" is called without a parameter specifying the format. # -# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite +# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite # with a different indexfile format version. If it isn't set the index # file format used is index-v[23]. # @@ -2599,8 +2599,8 @@ endif ifdef GIT_INTEROP_MAKE_OPTS @echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+ endif -ifdef TEST_GIT_INDEX_VERSION - @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+ +ifdef GIT_TEST_INDEX_VERSION + @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+ endif @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi diff --git a/t/README b/t/README index 9028b47d92..fb6359b17b 100644 --- a/t/README +++ b/t/README @@ -315,10 +315,13 @@ packs on demand. This normally only happens when the object size is over 2GB. This variable forces the code path on any object larger than <n> bytes. -GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon pack-objects code +GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncommon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_INDEX_VERSION=<n> exercises the index read/write code path +for the index version specified. Can be set to any valid version. + Naming Tests ------------ diff --git a/t/test-lib.sh b/t/test-lib.sh index 44288cbb59..31698c01c4 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -134,9 +134,9 @@ export EDITOR GIT_TRACE_BARE=1 export GIT_TRACE_BARE -if test -n "${TEST_GIT_INDEX_VERSION:+isset}" +if test -n "${GIT_TEST_INDEX_VERSION:+isset}" then - GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION" + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" export GIT_INDEX_VERSION fi base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670 -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support 2018-09-12 21:26 [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support Ben Peart 2018-09-12 22:31 ` Thomas Gummerer 2018-09-13 18:17 ` [PATCH v2] read-cache: update TEST_GIT_INDEX_VERSION support Ben Peart @ 2018-09-13 18:49 ` Ævar Arnfjörð Bjarmason 2018-09-13 20:59 ` Junio C Hamano 2 siblings, 1 reply; 10+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-09-13 18:49 UTC (permalink / raw) To: Ben Peart; +Cc: git@vger.kernel.org, gitster@pobox.com, Ben Peart On Wed, Sep 12 2018, Ben Peart wrote: > -GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon pack-objects code > +GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncommon pack-objects code > path where deltas larger than this limit require extra memory > allocation for bookkeeping. If re-rolled maybe better as a leading "fix a typo" patch. I was reading this in a mobile client earlier and couldn't see at a glance (small screen+wrapping) what the change was here, so that sort of nitpicking isn't purely theoretical :) > +GIT_TEST_INDEX_VERSION=<n> exercises the index read/write code path > +for the index version specified. Can be set to any valid version > +but the non-default version 4 is probably the most beneficial. > + I'm not familiar with this and haven't dug, so I don't know this: are any values of 1..4 OK? 0..4? Would be better to say that here... ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support 2018-09-13 18:49 ` [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support Ævar Arnfjörð Bjarmason @ 2018-09-13 20:59 ` Junio C Hamano 0 siblings, 0 replies; 10+ messages in thread From: Junio C Hamano @ 2018-09-13 20:59 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Ben Peart, git@vger.kernel.org, Ben Peart Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > On Wed, Sep 12 2018, Ben Peart wrote: > >> -GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon pack-objects code >> +GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncommon pack-objects code >> path where deltas larger than this limit require extra memory >> allocation for bookkeeping. > > If re-rolled maybe better as a leading "fix a typo" patch. Yeah, an easy to miss change that is unrelated to the topic is better made as a separate patch. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-09-14 16:20 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-12 21:26 [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support Ben Peart 2018-09-12 22:31 ` Thomas Gummerer 2018-09-13 14:07 ` Ben Peart 2018-09-13 21:56 ` Thomas Gummerer 2018-09-13 22:08 ` Junio C Hamano 2018-09-14 13:50 ` Ben Peart 2018-09-14 16:20 ` Junio C Hamano 2018-09-13 18:17 ` [PATCH v2] read-cache: update TEST_GIT_INDEX_VERSION support Ben Peart 2018-09-13 18:49 ` [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support Ævar Arnfjörð Bjarmason 2018-09-13 20:59 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).