git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/3] Add GIT_TEST_MULTI_PACK_INDEX environment variable
Date: Fri, 12 Oct 2018 10:34:16 -0700 (PDT)	[thread overview]
Message-ID: <pull.27.v2.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.27.git.gitgitgadget@gmail.com>

To increase coverage of the multi-pack-index feature, add a
GIT_TEST_MULTI_PACK_INDEX environment variable similar to other GIT_TEST_*
variables.

After creating the environment variable and running the test suite with it
enabled, I found a few bugs in the multi-pack-index implementation. These
are handled by the first two patches.

I have set up a CI build on Azure Pipelines [1] that runs the test suite
with a few optional features enabled, including GIT_TEST_MULTI_PACK_INDEX
and GIT_TEST_COMMIT_GRAPH. I'll use this to watch the features and ensure
they work well with the rest of the ongoing work. Eventually, we can add
these variables to the Travis CI scripts.

[1] https://git.visualstudio.com/git/_build?definitionId=4

Derrick Stolee (3):
  midx: fix broken free() in close_midx()
  midx: close multi-pack-index on repack
  multi-pack-index: define GIT_TEST_MULTI_PACK_INDEX

 builtin/repack.c            |  7 +++++--
 midx.c                      | 26 ++++++++++++++++++++------
 midx.h                      |  6 +++++-
 t/README                    |  4 ++++
 t/t5310-pack-bitmaps.sh     |  1 +
 t/t5319-multi-pack-index.sh |  2 +-
 t/t9300-fast-import.sh      |  2 +-
 7 files changed, 37 insertions(+), 11 deletions(-)


base-commit: 5a0cc8aca797dbd7d2be3b67458ff880ed45cddf
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-27%2Fderrickstolee%2Fmidx-test%2Fupstream-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-27/derrickstolee/midx-test/upstream-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/27

Range-diff vs v1:

 1:  9fcbbe336d = 1:  8bd672fe26 midx: fix broken free() in close_midx()
 2:  725ebadc92 ! 2:  2d8f26679d midx: close multi-pack-index on repack
     @@ -15,16 +15,15 @@
      --- a/builtin/repack.c
      +++ b/builtin/repack.c
      @@
     + 			char *fname, *fname_old;
       
       			if (!midx_cleared) {
     - 				/* if we move a packfile, it will invalidated the midx */
     -+				if (the_repository->objects) {
     -+					close_midx(the_repository->objects->multi_pack_index);
     -+					the_repository->objects->multi_pack_index = NULL;
     -+				}
     - 				clear_midx_file(get_object_directory());
     +-				/* if we move a packfile, it will invalidated the midx */
     +-				clear_midx_file(get_object_directory());
     ++				clear_midx_file(the_repository);
       				midx_cleared = 1;
       			}
     + 
      
      diff --git a/midx.c b/midx.c
      --- a/midx.c
     @@ -44,13 +43,34 @@
       	munmap((unsigned char *)m->data, m->data_len);
       	close(m->fd);
       	m->fd = -1;
     +@@
     + 	return 0;
     + }
     + 
     +-void clear_midx_file(const char *object_dir)
     ++void clear_midx_file(struct repository *r)
     + {
     +-	char *midx = get_midx_filename(object_dir);
     ++	char *midx = get_midx_filename(r->objects->objectdir);
     ++
     ++	if (r->objects && r->objects->multi_pack_index) {
     ++		close_midx(r->objects->multi_pack_index);
     ++		r->objects->multi_pack_index = NULL;
     ++	}
     + 
     + 	if (remove_path(midx)) {
     + 		UNLEAK(midx);
      
      diff --git a/midx.h b/midx.h
      --- a/midx.h
      +++ b/midx.h
      @@
     + int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local);
     + 
       int write_midx_file(const char *object_dir);
     - void clear_midx_file(const char *object_dir);
     +-void clear_midx_file(const char *object_dir);
     ++void clear_midx_file(struct repository *r);
     + int verify_midx_file(const char *object_dir);
       
      +void close_midx(struct multi_pack_index *m);
      +
 3:  04e3e91082 = 3:  57c64e814c multi-pack-index: define GIT_TEST_MULTI_PACK_INDEX

-- 
gitgitgadget

  parent reply	other threads:[~2018-10-12 17:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-08 15:17 [PATCH 0/3] Add GIT_TEST_MULTI_PACK_INDEX environment variable Derrick Stolee via GitGitGadget
2018-10-08 15:17 ` [PATCH 1/3] midx: fix broken free() in close_midx() Derrick Stolee via GitGitGadget
2018-10-09  9:07   ` Junio C Hamano
2018-10-08 15:17 ` [PATCH 2/3] midx: close multi-pack-index on repack Derrick Stolee via GitGitGadget
2018-10-09  9:10   ` Junio C Hamano
2018-10-09 14:11     ` Derrick Stolee
2018-10-09 18:15       ` Stefan Beller
2018-10-08 15:17 ` [PATCH 3/3] multi-pack-index: define GIT_TEST_MULTI_PACK_INDEX Derrick Stolee via GitGitGadget
2018-10-12 17:34 ` Derrick Stolee via GitGitGadget [this message]
2018-10-12 17:34   ` [PATCH v2 1/3] midx: fix broken free() in close_midx() Derrick Stolee via GitGitGadget
2018-10-12 17:34   ` [PATCH v2 2/3] midx: close multi-pack-index on repack Derrick Stolee via GitGitGadget
2018-10-12 17:34   ` [PATCH v2 3/3] multi-pack-index: define GIT_TEST_MULTI_PACK_INDEX Derrick Stolee via GitGitGadget
2018-10-12 17:41   ` [PATCH v2 0/3] Add GIT_TEST_MULTI_PACK_INDEX environment variable Derrick Stolee
2018-10-22  1:41     ` Junio C Hamano

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=pull.27.v2.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).