git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Antonio Ospite <ao2@ao2.it>
To: gitster@pobox.com
Cc: git@vger.kernel.org, Brandon Williams <bmwill@google.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Stefan Beller <sbeller@google.com>, Jeff King <peff@peff.net>,
	SZEDER Gábor <szeder.dev@gmail.com>, Antonio Ospite <ao2@ao2.it>
Subject: [PATCH v6 00/10] Make submodules work if .gitmodules is not checked out
Date: Fri,  5 Oct 2018 15:05:51 +0200
Message-ID: <20181005130601.15879-1-ao2@ao2.it> (raw)

Hi,

this series teaches git to try and read the .gitmodules file from the
index (:.gitmodules) or from the current branch (HEAD:.gitmodules) when
the file is not readily available in the working tree.

This can be used, together with sparse checkouts, to enable submodule
usage with programs like vcsh[1] which manage multiple repositories with
their working trees sharing the same path.

[1] https://github.com/RichiH/vcsh


Thanks to SZEDER Gábor we found out that the changes in patch 9 could
allow to access the object store in an inconsistent way when using
multi-threading in "git grep --recurse-submodules", this has been dealt
with in this revision.

BTW, with Stefan Beller we also identified some unneeded code which
could have been removed to alleviate the issue, but that would not have
solved it completely; so, I am not removing the unnecessary call to
repo_read_gitmodules() builtin/grep.c in this series, possibly this can
become a stand-alone change.

The problems from above also made me investigate what implications the
current use of a global object store had on my new addition, and now
I know that there is one case which I cannot support just yet: nested
submodules without .gitmodules in their working tree.

This case has been documented with a warning and test_expect_failure
items in tests, and hitting the unsupported case does not alter the
current behavior of git.

Apart form patch 9 and 10 there are no major changes to the previous
iteration.

IMHO we are in a place where the problem has been analyzed with enough
depth, the limitations have been identified and dealt with in a way that
should not affect current users nor diminish the usefulness of the new
code.

v5 of the series is here:
https://public-inbox.org/git/20180917140940.3839-1-ao2@ao2.it/

v4 of the series is here:
https://public-inbox.org/git/20180824132951.8000-1-ao2@ao2.it/

v3 of the series is here:
https://public-inbox.org/git/20180814110525.17801-1-ao2@ao2.it/

v2 of the series is here:
https://public-inbox.org/git/20180802134634.10300-1-ao2@ao2.it/

v1 of the series, with some background info, is here:
https://public-inbox.org/git/20180514105823.8378-1-ao2@ao2.it/

Changes since v5:

  * print_config_from_gitmodules() in patch 1 now accepts a struct
    repository argument.

  * submodule--helper config in patch 5 has been adjusted to use the new
    signature of print_config_from_gitmodules().

  * In patch 9 the grep read lock in builtin/grep.c now covers all code
    paths involving config_from_gitmodules().
    
    FTR git-grep is the only place where config_from_gitmodules() is
    called from multi-threaded code.

  * Patch 9 also documents the rare case that cannot be supported just
    yet, and adds a warning to the user.

  * In patch 9, config_from_gitmodules() now does not read any config
    when the config source is not specified.(I added a catch-all "else"
    block) This match more closely the behavior of the old code using
    git_config_from_file.

  * Added a new test tool in patch 10 to exercise config_read_config()
    in a more direct way, passing an arbitrary repository.
    
    Admittedly, patch 10 performs a similar test to the one added to
    t7814 in patch 9, so I'd be OK with dropping patch 10 if it is too
    specific.

Thank you,
   Antonio

Antonio Ospite (10):
  submodule: add a print_config_from_gitmodules() helper
  submodule: factor out a config_set_in_gitmodules_file_gently function
  t7411: merge tests 5 and 6
  t7411: be nicer to future tests and really clean things up
  submodule--helper: add a new 'config' subcommand
  submodule: use the 'submodule--helper config' command
  t7506: clean up .gitmodules properly before setting up new scenario
  submodule: add a helper to check if it is safe to write to .gitmodules
  submodule: support reading .gitmodules when it's not in the working
    tree
  t/helper: add test-submodule-nested-repo-config

 Makefile                                     |   1 +
 builtin/grep.c                               |  17 ++-
 builtin/submodule--helper.c                  |  40 ++++++
 cache.h                                      |   2 +
 git-submodule.sh                             |  13 +-
 submodule-config.c                           |  68 ++++++++-
 submodule-config.h                           |   2 +
 submodule.c                                  |  28 +++-
 submodule.h                                  |   1 +
 t/helper/test-submodule-nested-repo-config.c |  30 ++++
 t/helper/test-tool.c                         |   1 +
 t/helper/test-tool.h                         |   1 +
 t/t7411-submodule-config.sh                  | 141 +++++++++++++++++--
 t/t7416-submodule-sparse-gitmodules.sh       |  78 ++++++++++
 t/t7506-status-submodule.sh                  |   3 +-
 t/t7814-grep-recurse-submodules.sh           |  16 +++
 16 files changed, 410 insertions(+), 32 deletions(-)
 create mode 100644 t/helper/test-submodule-nested-repo-config.c
 create mode 100755 t/t7416-submodule-sparse-gitmodules.sh

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

             reply index

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-05 13:05 Antonio Ospite [this message]
2018-10-05 13:05 ` [PATCH v6 01/10] submodule: add a print_config_from_gitmodules() helper Antonio Ospite
2018-10-05 13:05 ` [PATCH v6 02/10] submodule: factor out a config_set_in_gitmodules_file_gently function Antonio Ospite
2018-10-05 13:05 ` [PATCH v6 03/10] t7411: merge tests 5 and 6 Antonio Ospite
2018-10-05 13:05 ` [PATCH v6 04/10] t7411: be nicer to future tests and really clean things up Antonio Ospite
2018-10-05 13:05 ` [PATCH v6 05/10] submodule--helper: add a new 'config' subcommand Antonio Ospite
2018-10-05 13:05 ` [PATCH v6 06/10] submodule: use the 'submodule--helper config' command Antonio Ospite
2018-10-05 13:05 ` [PATCH v6 07/10] t7506: clean up .gitmodules properly before setting up new scenario Antonio Ospite
2018-10-05 13:05 ` [PATCH v6 08/10] submodule: add a helper to check if it is safe to write to .gitmodules Antonio Ospite
2018-10-05 23:50   ` Stefan Beller
2018-10-06  9:19     ` Antonio Ospite
2018-10-06 23:44     ` Junio C Hamano
2018-10-08 12:37       ` Antonio Ospite
2018-10-05 13:06 ` [PATCH v6 09/10] submodule: support reading .gitmodules when it's not in the working tree Antonio Ospite
2018-10-08 22:19   ` Stefan Beller
2018-10-10 18:56     ` Antonio Ospite
2018-10-10 22:55       ` Stefan Beller
2018-10-09  3:39   ` Junio C Hamano
2018-10-09  3:48     ` Junio C Hamano
2018-10-05 13:06 ` [PATCH v6 10/10] t/helper: add test-submodule-nested-repo-config Antonio Ospite
2018-10-06  9:20 ` [PATCH v6 00/10] Make submodules work if .gitmodules is not checked out Antonio Ospite
2018-10-25  8:40 ` Junio C Hamano
2018-10-25 13:20   ` Antonio Ospite

Reply instructions:

You may reply publically 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20181005130601.15879-1-ao2@ao2.it \
    --to=ao2@ao2.it \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    --cc=szeder.dev@gmail.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

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox