git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: David Turner <David.Turner@twosigma.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [PATCH] submodules: allow empty working-tree dirs in merge/cherry-pick
Date: Mon, 7 Nov 2016 12:48:22 -0800	[thread overview]
Message-ID: <CAGZ79kb8XM_j37H-j3fUz9a0DFXn_v+S4P4r-QyuQOKM3xqhRw@mail.gmail.com> (raw)
In-Reply-To: <378e63aa70e54fe9b839acf90680917a@exmbdft7.ad.twosigma.com>

On Mon, Nov 7, 2016 at 12:38 PM, David Turner <David.Turner@twosigma.com> wrote:
>> -----Original Message-----
>> From: Stefan Beller [mailto:sbeller@google.com]
>> Sent: Monday, November 07, 2016 2:14 PM
>> To: David Turner
>> Cc: git@vger.kernel.org
>> Subject: Re: [PATCH] submodules: allow empty working-tree dirs in
>> merge/cherry-pick
>>
>> On Mon, Nov 7, 2016 at 10:31 AM, David Turner <dturner@twosigma.com>
>> wrote:
>> > When a submodule is being merged or cherry-picked into a working tree
>> > that already contains a corresponding empty directory, do not record a
>> > conflict.
>> >
>> > One situation where this bug appears is:
>> >
>> > - Commit 1 adds a submodule
>>
>> "... at sub1" as inferred by text below.
>>
>> > - Commit 2 removes that submodule and re-adds it into a subdirectory
>> >        (sub1 to sub1/sub1).
>> > - Commit 3 adds an unrelated file.
>> >
>> > Now the user checks out commit 1 (first deinitializing the submodule),
>> > and attempts to cherry-pick commit 3.  Previously, this would fail,
>> > because the incoming submodule sub1/sub1 would falsely conflict with
>> > the empty sub1 directory.
>>
>> So you'd want to achieve:
>>   $ # on commit 3:
>>   git checkout <commit 1>
>>   git cherry-pick <commit 3>
>>
>> which essentially moves the gitlink back to its original place (from
>> sub1/sub1 -> sub1).  This sounds reasonable.
>> But what if the submodule contains a (file/directory) named sub1? We'd
>> first remove the sub1/sub1 submodule (and even delete the inner
>> directory?), such that "sub1/"
>> becomes an empty dir, which is perfect for having a submodule right there
>> at "sub1/"
>
> I'm confused about the "what if" here.
>
> In our particular situation, the submodule in question was not initialized.

oops. That explains it. I somehow assumed we were talking about
initialized submodules.

>
> If your approach also fixes the same tests that mine fixes, then I am happy to use your series over mine.  Please CC me so I can take a peek.

No, my series seems to be orthogonal to this one. I plan
on cc'ing you nevertheless as it is still nearby.

> basically nobody needs two copies of one submodule in the same repo.

IIRC this is how gitlinks were used in very early days :/
(kernel people were using gitlinks to track different kernel versions
and see if they were interoperable or working at all.
e.g. see d92a39590d1126e195f1bbccf182a2cdb79218e7, which
only makes sense (for the update command) if the referenced repository
contains references of all submodules, which either means a huge reference
pile that contains different projects at the same time, or the same project
at different versions.

>  I think that case fails for other reasons anyway.
>

Yes. I agree that the patch as-is is applicable. I did not try to oppose
your approach, but rather give some thoughts I had.

Stefan

  reply	other threads:[~2016-11-07 20:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-07 18:31 [PATCH] submodules: allow empty working-tree dirs in merge/cherry-pick David Turner
2016-11-07 19:13 ` Stefan Beller
2016-11-07 20:38   ` David Turner
2016-11-07 20:48     ` Stefan Beller [this message]
2016-11-18  4:47 ` 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

  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=CAGZ79kb8XM_j37H-j3fUz9a0DFXn_v+S4P4r-QyuQOKM3xqhRw@mail.gmail.com \
    --to=sbeller@google.com \
    --cc=David.Turner@twosigma.com \
    --cc=git@vger.kernel.org \
    /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).