git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Antonio Ospite <ao2@ao2.it>
To: git@vger.kernel.org
Cc: Brandon Williams <bmwill@google.com>,
	Daniel Graña <dangra@gmail.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Richard Hartmann <richih.mailinglist@gmail.com>,
	Stefan Beller <sbeller@google.com>, Antonio Ospite <ao2@ao2.it>
Subject: [RFC PATCH v2 00/12] Make submodules work if .gitmodules is not checked out
Date: Thu,  2 Aug 2018 15:46:22 +0200
Message-ID: <20180802134634.10300-1-ao2@ao2.it> (raw)

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

-- 
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: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-02 13:46 Antonio Ospite [this message]
2018-08-02 13:46 ` [RFC PATCH v2 01/12] submodule: add a print_config_from_gitmodules() helper Antonio Ospite
2018-08-02 18:05   ` Stefan Beller
2018-08-09 10:17     ` Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 02/12] submodule: factor out a config_set_in_gitmodules_file_gently function Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 03/12] t7411: be nicer to future tests and really clean things up Antonio Ospite
2018-08-02 16:40   ` SZEDER Gábor
2018-08-02 18:15     ` Stefan Beller
2018-08-09 13:59       ` Antonio Ospite
2018-08-02 18:44     ` Junio C Hamano
2018-08-02 13:46 ` [RFC PATCH v2 04/12] submodule--helper: add a new 'config' subcommand Antonio Ospite
2018-08-02 18:47   ` Stefan Beller
2018-08-02 19:20     ` Jeff King
2018-08-03 10:21       ` Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 05/12] submodule: use the 'submodule--helper config' command Antonio Ospite
2018-08-02 18:59   ` Stefan Beller
2018-08-02 13:46 ` [RFC PATCH v2 06/12] submodule--helper: add a '--stage' option to the 'config' sub command Antonio Ospite
2018-08-02 18:57   ` Junio C Hamano
2018-08-03 11:03     ` Antonio Ospite
2018-08-03 16:24       ` Junio C Hamano
2018-08-06 10:58         ` Antonio Ospite
2018-08-06 17:38           ` Junio C Hamano
2018-08-07  9:19             ` Antonio Ospite
2018-08-06 18:19           ` Stefan Beller
2018-08-02 13:46 ` [RFC PATCH v2 07/12] submodule: use 'submodule--helper config --stage' to stage .gitmodules Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 08/12] t7506: cleanup .gitmodules properly before setting up new scenario Antonio Ospite
2018-08-02 19:11   ` Stefan Beller
2018-08-02 13:46 ` [RFC PATCH v2 09/12] submodule: support reading .gitmodules even when it's not checked out Antonio Ospite
2018-08-02 20:27   ` Stefan Beller
2018-08-02 13:46 ` [RFC PATCH v2 10/12] t7416: add new test about HEAD:.gitmodules and not existing .gitmodules Antonio Ospite
2018-08-02 20:43   ` Stefan Beller
2018-08-09  9:14     ` Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 11/12] dir: move is_empty_file() from builtin/am.c to dir.c and make it public Antonio Ospite
2018-08-02 20:50   ` Stefan Beller
2018-08-03  8:49     ` Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 12/12] submodule: remove the .gitmodules file when it is empty Antonio Ospite
2018-08-02 21:15   ` Stefan Beller
2018-08-03  8:57     ` 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=20180802134634.10300-1-ao2@ao2.it \
    --to=ao2@ao2.it \
    --cc=bmwill@google.com \
    --cc=dangra@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=richih.mailinglist@gmail.com \
    --cc=sbeller@google.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 list mirror (unofficial, 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/

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