[RFC PATCH v2 00/12] Make submodules work if .gitmodules is not checked out

Hi,

this is a second version of the RFC from https://public-inbox.org/git/20180514105823.8378-1-ao2@ao2.it/

Please refer to the cover letter of the previous series for the motivation and background of the changes.

Patches from 01 to 08 should be rather uncontroversial, as they do not introduce any change of behavior but just prepare the ground for patches 09 and 10.

The last two patches, 11 and 12 are not strictly related to the topic, it's just an idea I came up with while reading the code for the series; I could send them separately if you think they are valid, or drop them.

The behavior of accessing HEAD:.gitmodules have been analyzed with the following test matrix:

Predicates:
  G = .gitmodules exists
  H = HEAD:.gitmodules exists

Operations:
  read = git submodule--helper config name
  write = git submodule--helper config name value
  stage = git submodule--helper config --stage

!G and !H:
  - read: nop
  - write: set value to .gitmodules (we are creating a brand new
    .gitmodules)
  - stage: exit with error, staging requires .gitmodules (like git
    add/rm)

G and !H:
  - read: get value from from .gitmodules
  - write: set value to .gitmodules
  - stage: stage .gitmodules

!G and H:
  - read: get the value from HEAD:.gitmodules
  - write: exit with error, setting a value requires .gitmodules
  - stage: exit with error, staging requires .gitmodules
  
G and H:
  - read: get value from from .gitmodules
  - write: set value to .gitmodules
  - stage: stage .gitmodules

Note that "git rm" and "git mv" will keep working when !G just as t3600-rm.sh and t7001-mv.sh mandate, I am not sure if they should fail if "!G and H".

In my previous attempt I also tried to check for the skip-worktree bit, but I am not sure if this is needed.

My intuition suggested the following behavior to cover the case of a manually crafted .gitmodules which could be committed and override the hidden one:

S = .gitmodules has the skip-worktree bit set

S:
  - read: get value from .gitmodules if G or from HEAD:.gitmodules if H
  - write: exit with error, .gitmodules is not supposed to be changed
  - stage: exit with error, .gitmodules is not supposed to be changed

However it looks like git gives the user quite some freedom to add, stage, commit, remove files even when they have the skip-worktree bit set (which was a little surprising to me TBH) so I am leaving this out for now.

Changes since v1:

  * Added a helper to print config values from the gitmodules
    configuration, the code is now in submoudle-config.c instead of
    buildtin/submodule--helper.c

  * Renamed config_gitmodules_set to
    config_set_in_gitmodules_file_gently and put it in
    submodule-config.c instead of submodule.c

    The naming follows the style of git config functions, and also takes
    into account Stefan Beller's comment about the final purpose of the
    function: we only write gitmodules config to the actual .gitmodules
    file, so we may as well reflect that in the function name,

  * Started using "working tree" instead of "work tree" as the former is
    the official term for the set of checked out files.

  * Start referring to HEAD as the "current branch" instead of "the
    index", the former should be more accurate.

  * Renamed GITMODULES_BLOB to GITMODULES_HEAD because GITMODULES_BLOB
    was added in commit 59e7b080 ("fsck: detect gitmodules files",
    2018-05-02)

  * Do not check for the skip-worktree bit on .gitmodules anymore, just
    check for file existence when appropriate.

  * Renamed t7415-submodule-sparse-gitmodules.sh to
    t7416-submodule-sparse-gitmodules.sh because t7415 was taken by
    t7415-submodule-names.sh

  * Made "git mv" and "git rm" work again when .gitmodules does not
    exist.

  * Removed checks about the  skip-worktree bit.

Thanks,
   Antonio

Antonio Ospite (12):
  submodule: add a print_config_from_gitmodules() helper
  submodule: factor out a config_set_in_gitmodules_file_gently function
  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
  submodule--helper: add a '--stage' option to the 'config' sub command
  submodule: use 'submodule--helper config --stage' to stage .gitmodules
  t7506: cleanup .gitmodules properly before setting up new scenario
  submodule: support reading .gitmodules even when it's not checked out
  t7416: add new test about HEAD:.gitmodules and not existing
    .gitmodules
  dir: move is_empty_file() from builtin/am.c to dir.c and make it
    public
  submodule: remove the .gitmodules file when it is empty

 builtin/am.c                           |  15 ----
 builtin/submodule--helper.c            |  82 ++++++++++++++++++
 cache.h                                |   1 +
 dir.c                                  |  16 ++++
 dir.h                                  |   1 +
 git-submodule.sh                       |  10 +--
 submodule-config.c                     |  53 +++++++++++-
 submodule-config.h                     |   3 +
 submodule.c                            |  20 +++--
 t/t3600-rm.sh                          |  32 +++----
 t/t7400-submodule-basic.sh             |  11 +++
 t/t7411-submodule-config.sh            |  65 +++++++++++++-
 t/t7416-submodule-sparse-gitmodules.sh | 112 +++++++++++++++++++++++++
 t/t7506-status-submodule.sh            |   3 +-
 14 files changed, 375 insertions(+), 49 deletions(-)
 create mode 100755 t/t7416-submodule-sparse-gitmodules.sh