git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Brandon Williams <bmwill@google.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Jens Lehmann <Jens.Lehmann@web.de>,
	Heiko Voigt <hvoigt@hvoigt.net>
Subject: Re: [PATCH 3/3] submodule--helper: add intern-git-dir function
Date: Mon, 21 Nov 2016 18:09:41 -0800	[thread overview]
Message-ID: <CAGZ79kb_4wWs_90AfsT932iPWbCXf6yRq875JUxoRZjUcsBW5A@mail.gmail.com> (raw)
In-Reply-To: <xmqqy40ch6wp.fsf@gitster.mtv.corp.google.com>

On Mon, Nov 21, 2016 at 1:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Does this format correctly?
>
> I somehow thought that second and subsequent paragraphs continued
> with "+" want no indentation before them.  See for example the
> Values section in config.txt and see how entries for boolean:: and
> color:: use multiple '+' paragraphs.
>
> If we do not have to refrain from indenting the second and
> subsequent paragraphs, that would be great for readability, but I
> take the existing practice as telling me that we cannot do that X-<.

Will fix and test in a resend.


>
>> +test_expect_success 'setup a gitlink with missing .gitmodules entry' '
>> +     git init sub2 &&
>> +     test_commit -C sub2 first &&
>> +     git add sub2 &&
>> +     git commit -m superproject
>> +'
>> +
>> +test_expect_success 'intern the git dir fails for incomplete submodules' '
>> +     test_must_fail git submodule interngitdirs &&
>> +     # check that we did not break the repository:
>> +     git status
>> +'
>
> It is not clear what the last "git status" wants to test.

Any errors that I ran into when manually truing to embed a submodules
git dir, were fatal with `git status` already, e.g. missing or wrong
call of connect_work_tree_and_git_dir were my main failure points.

So I guess we should test a bit more extensively, maybe

    git status >expect
    git submodule embedgitdirs
    git status >actual
    test_cmp expect actual
    # further testing via
    test -f ..
    test -d ..

>  In the
> extreme, if the failed "git submodule" command did
>
>         rm -fr .git ?* && git init
>
> wouldn't "git status" still succeed?

    In that particular case you'd get
    $ git status
    fatal: Not a git repository (or any parent up to mount point ....)
    Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
    $ echo $?
    128

but I get the idea, which is why I propose the double check via status.
That would detect any logical change for the repository, e.g. a
change to the .gitmodules file.

>
> What are the minimum things that we expect from "did not break" to
> see?  sub2/.git is still a directory and is a valid repository?  The
> contents of the .git/modules/* before and after the "git submodule"
> does not change?  Some other things?

I thought about making up a name for such a repo and creating that engry
in .gitmodules. But I refrained from doing so, because it seems too much
for this command.

I dunno, but I would suspect the double status is fine here, too?

  reply	other threads:[~2016-11-22  2:09 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-21 20:41 [PATCH 0/3] Introduce `submodule interngitdirs` Stefan Beller
2016-11-21 20:41 ` [PATCH 1/3] submodule: use absolute path for computing relative path connecting Stefan Beller
2016-11-21 21:01   ` Junio C Hamano
2016-11-21 21:03     ` Stefan Beller
2016-11-22  0:04       ` Stefan Beller
2016-11-22  7:02         ` Junio C Hamano
2016-11-21 20:41 ` [PATCH 2/3] test-lib-functions.sh: teach test_commit -C <dir> Stefan Beller
2016-11-21 21:04   ` Junio C Hamano
2016-11-21 20:41 ` [PATCH 3/3] submodule--helper: add intern-git-dir function Stefan Beller
2016-11-21 21:14   ` Junio C Hamano
2016-11-22  2:09     ` Stefan Beller [this message]
2016-11-22  7:07       ` Junio C Hamano
2016-11-22 17:16         ` Stefan Beller
2016-11-22 17:53           ` Junio C Hamano
2016-11-21 21:56   ` Brandon Williams
2016-11-21 20:48 ` [PATCH 0/3] Introduce `submodule interngitdirs` Junio C Hamano
2016-11-21 20:56   ` Stefan Beller

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

  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=CAGZ79kb_4wWs_90AfsT932iPWbCXf6yRq875JUxoRZjUcsBW5A@mail.gmail.com \
    --to=sbeller@google.com \
    --cc=Jens.Lehmann@web.de \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hvoigt@hvoigt.net \
    --cc=jrnieder@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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).