Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Brandon Williams]

On 10/11, Josh Triplett wrote:
> On Tue, Oct 10, 2017 at 11:39:21AM -0700, Stefan Beller wrote:
> > On Tue, Oct 10, 2017 at 6:03 AM, Heiko Voigt  wrote:
> > > but in the long run my goal
> > > for submodules is and always was: Make them behave as close to files as
> > > possible. And why should a 'git add submodule' not magically do
> > > everything it can to make submodules just work? I can look into a patch
> > > for that if people agree here...
> > 
> > I'd love to see this implemented. I cc'd Josh (the author of git-series), 
> > who
> > may disagree with this, or has some good input how to go forward without
> > breaking git-series.
> 
> git-series doesn't use the git-submodule command at all, nor does it
> construct series trees using git-add or any other git command-line tool;
> it constructs gitlinks directly. Most of the time, it doesn't even make
> sense to `git checkout` a series branch. Modifying commands like git-add
> and similar to automatically manage .gitmodules won't cause any issue at
> all, as long as git itself doesn't start rejecting or complaining about
> repositories that have gitlinks without a .gitmodules file.

That's good to know!  And from what I remember, with the commands we've
begun teaching to understand submodules we have been requiring a
.gitmodules entry for a submodule in order to do the recursion, and a
gitlink without a .gitmodules entry would simply be ignored or skipped.

-- 
Brandon Williams

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Junio C Hamano]

Heiko Voigt  writes:

> This "default" value thing got me thinking in a different direction. We
> could use a scheme like that to get names (and values) for submodules
> that are missing from the .gitmodules file. If we decide that we need to
> handle them.

Yes, I suspect that would improve things quite a bit in a repository
where it added a new submodule by filling the gap between the time
when a gitlink is added and an entry in .gitmodules is added.  The
latter needs to happen if the result of the work done in that
repository is pushed out elsewhere---otherwise it won't usable by
other people.

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Josh Triplett]

On Tue, Oct 10, 2017 at 11:39:21AM -0700, Stefan Beller wrote:
> On Tue, Oct 10, 2017 at 6:03 AM, Heiko Voigt  wrote:
> > but in the long run my goal
> > for submodules is and always was: Make them behave as close to files as
> > possible. And why should a 'git add submodule' not magically do
> > everything it can to make submodules just work? I can look into a patch
> > for that if people agree here...
> 
> I'd love to see this implemented. I cc'd Josh (the author of git-series), who
> may disagree with this, or has some good input how to go forward without
> breaking git-series.

git-series doesn't use the git-submodule command at all, nor does it
construct series trees using git-add or any other git command-line tool;
it constructs gitlinks directly. Most of the time, it doesn't even make
sense to `git checkout` a series branch. Modifying commands like git-add
and similar to automatically manage .gitmodules won't cause any issue at
all, as long as git itself doesn't start rejecting or complaining about
repositories that have gitlinks without a .gitmodules file.

- Josh Triplett

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Heiko Voigt]

On Wed, Oct 11, 2017 at 08:31:37AM +0900, Junio C Hamano wrote:
> Stefan Beller  writes:
> 
> > So you propose to make git-add behave like "git submodule add"
> > (i.e. also add the .gitmodules entry for name/path/URL), which I
> > like from a submodule perspective.
> >
> > However other users of gitlinks might be confused[1], which is why
> > I refrained from "making every gitlink into a submodule". Specifically
> > the more powerful a submodule operation is (the more fluff adds),
> > the harder it should be for people to mis-use it.
> 
> A few questions that come to mind are:
> 
>  - Does "git add sub/" have enough information to populate
>.gitmodules?  If we have reasonable "default" values for
>.gitmodules entries (e.g. missing URL means we won't fetch when
>asked to go recursively fetch), perhaps we can leave everything
>other than "submodule.$name.path" undefined.

My suggestion would be: If we do not have them we do not populate them.
We could even go further and say: If we do not have the set "git
submodule add" would populate then we do not add anything to .gitmodules
and warn the user.

>  - Can't we help those who have gitlinks without .gitmodules entries
>exactly the same way as above, i.e. when we see a gitlink and try
>to treat it as a submodule, we'd first try to look it up from
>.gitmodules (by going from path to name and then to
>submodule.$name.$var); the above "'git add sub/' would add an
>entry for .gitmodules" wish is based on the assumption that there
>are reasonable "default" values for each of these $var--so by
>basing on the same assumption, we can "pretend" as if these
>submodule.$name.$var were in .gitmodules file when we see
>gitlinks without .gitmodules entries.  IOW, if "git add sub/" can
>add .gitmodules to help people without having to type "git
>submodule add sub/", then we can give exactly the same degree of
>help without even modifying .gitmodules when "git add sub/" is
>run.

This "default" value thing got me thinking in a different direction. We
could use a scheme like that to get names (and values) for submodules
that are missing from the .gitmodules file. If we decide that we need to
handle them.

Cheers Heiko

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Heiko Voigt]

On Tue, Oct 10, 2017 at 11:39:21AM -0700, Stefan Beller wrote:
> So you propose to make git-add behave like "git submodule add"
> (i.e. also add the .gitmodules entry for name/path/URL), which I
> like from a submodule perspective.

Well more like: clone and add will behave like "git submodule add" but
basically yes.

> However other users of gitlinks might be confused[1], which is why
> I refrained from "making every gitlink into a submodule". Specifically
> the more powerful a submodule operation is (the more fluff adds),
> the harder it should be for people to mis-use it.
> 
> [1] https://github.com/git-series/git-series/blob/master/INTERNALS.md
>  "git-series uses gitlinks to store pointer to commits in its own repo."

But would those users use

git add

to add a gitlink? From the description in that file I read that it
points to commits in its own repository. Will there also be files
checked out like submodules at that location?

Otherwise I would propose that 'git add' could detect whether a gitlink
is a submodule by trying to read its git configuration. If we do not
find that we simply do not do anything.

> > If everyone agrees that submodules are the default way of handling
> > repositories insided repositories, IMO, 'git add' should also alter
> > .gitmodules by default. We could provide a switch to avoid doing that.
> 
> I wonder if that switch should be default-on (i.e. not treat a gitlink as
> a submodule initially, behavior as-is, and then eventually we will
> die() on unconfigured repos, expecting the user to make the decision)
> 
> > An intermediate solution would be to warn
> 
> That is already implemented by Peff.

Ah ok, thanks I suspected so when I realized that this discussion was
older.

> > but in the long run my goal
> > for submodules is and always was: Make them behave as close to files as
> > possible. And why should a 'git add submodule' not magically do
> > everything it can to make submodules just work? I can look into a patch
> > for that if people agree here...
> 
> I'd love to see this implemented. I cc'd Josh (the author of git-series), who
> may disagree with this, or has some good input how to go forward without
> breaking git-series.

Yeah, lets see if, as described above, that actually would break
git-series.

> > Regarding handling of gitlinks with or without .gitmodules:
> >
> > Currently we are actually in some intermediate state:
> >
> >  * If there is no .gitmodules file: No submodule processing on any
> >gitlinks (AFAIK)
> 
> AFAIK this is true.
> 
> >  * If there is a .gitmodules files with some submodule configured: Do
> >recursive fetch and push as far as possible on gitlinks.
> 
> * If submodule.recurse is set, then we also treat submodules like files
>   for checkout, reset, read-tree.

To clarify: If submodule.recurse is set but there is no .gitmodules file
we do submodule processing for the above commands?

> > So I am not sure whether there are actually many users (knowingly)
> > using a mix of some submodules configured and some not and then relying
> > on the submodule infrastructure.
> >
> > I would rather expect two sorts of users:
> >
> >   1. Those that do use .gitmodules
> 
> Those want to reap all benefits of good submodules.
> 
> >
> >   2. Those that do *not* use .gitmodules
> 
> As said above, we don't know if those users are
> "holding submodules wrong" or are using gitlinks for
> magic tricks (unrelated to submodules).

I did not want to say that they are "holding submodules wrong" but
rather that if they do not use .gitmodules they do that knowingly and
thus consistently not use .gitmodules for any gitlink.

> > Users that do not use any .gitmodules file will currently (AFAIK) not
> > get any submodule handling. So the question is are there really many
> > "mixed users"? My guess would be no.
> 
> I hope that there are few (if any) users of these mixed setups.

That sounds promising.

> > Because without those using this mixed we could switch to saying: "You
> > need to have a .gitmodules file for submodule handling" without much
> > fallout from breaking users use cases.
> 
> That seems reasonable to me, actually.

Nice.

> > Maybe we can test this out somehow? My patch series would be ready in
> > that case, just had to drop the first patch and adjust the commit
> > message of this one.
> 
> I wonder how we would test this, though? Do you have any idea
> (even vague) how we'd accomplish such a measurement?
> I fear we'll have to go this way blindly.

One idea would be to expose this somewhere to a limited amount of users.
I remember Jonathan was suggesting, back when Jens was working on the
recursive checkout, that he could add the series to the debian package
and see what happens. Or we could use Junios next branch? Something like
that. If we get complaints we know the assumption was wrong and we need
a fallback.

Cheers Heiko

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Junio C Hamano]

Junio C Hamano  writes:

> A few questions that come to mind are:
>
>  - Does "git add sub/" have enough information to populate
>.gitmodules?  If we have reasonable "default" values for
>.gitmodules entries (e.g. missing URL means we won't fetch when
>asked to go recursively fetch), perhaps we can leave everything
>other than "submodule.$name.path" undefined.
> ...
>  - ...  IOW, if "git add sub/" can
>add .gitmodules to help people without having to type "git
>submodule add sub/", then we can give exactly the same degree of
>help without even modifying .gitmodules when "git add sub/" is
>run.

Answering my own questions (aka correcting my own stupidity), there
is a big leap/gap between the two that came from my forgetting an
important point: a local repository has a lot richer information
than others that are clones of it.

"git add sub/" could look at sub/.git/config and use that
information when considering what values to populate .gitmodules
with.  It can learn where its origin remote is, for example.

And while this can do that at look-up time locally (i.e. removing
the need to do .gitmodules), those who pull from this local
repository, of those who pull from a shared central repository this
local repository pushes into, will not have the same information
available to them, _unless_ this local repository records it in the
.gitmodules file for them to use.

So, I think "git add sub/" that adds to .gitmodules would work
(unless the sub/ repository originates locally without pushing
out--in which case, submodule.$name.url cannot be populated with a
value suitable for other people, and we should continue warning),
while doing the same at look-up time would not be a good solution.

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Stefan Beller]

On Tue, Oct 10, 2017 at 4:31 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> So you propose to make git-add behave like "git submodule add"
>> (i.e. also add the .gitmodules entry for name/path/URL), which I
>> like from a submodule perspective.
>>
>> However other users of gitlinks might be confused[1], which is why
>> I refrained from "making every gitlink into a submodule". Specifically
>> the more powerful a submodule operation is (the more fluff adds),
>> the harder it should be for people to mis-use it.
>
> A few questions that come to mind are:
>
>  - Does "git add sub/" have enough information to populate
>.gitmodules?  If we have reasonable "default" values for
>.gitmodules entries (e.g. missing URL means we won't fetch when
>asked to go recursively fetch), perhaps we can leave everything
>other than "submodule.$name.path" undefined.

I think we would want to populate path and URL only.

>
>  - Can't we help those who have gitlinks without .gitmodules entries
>exactly the same way as above, i.e. when we see a gitlink and try
>to treat it as a submodule, we'd first try to look it up from
>.gitmodules (by going from path to name and then to
>submodule.$name.$var); the above "'git add sub/' would add an
>entry for .gitmodules" wish is based on the assumption that there
>are reasonable "default" values for each of these $var--so by
>basing on the same assumption, we can "pretend" as if these
>submodule.$name.$var were in .gitmodules file when we see
>gitlinks without .gitmodules entries.  IOW, if "git add sub/" can
>add .gitmodules to help people without having to type "git
>submodule add sub/", then we can give exactly the same degree of
>help without even modifying .gitmodules when "git add sub/" is
>run.

I do not understand the gist of this paragraph, other then:

  "When git-add  encounters a section submodule..*,
   do not modify it; We can assume it is sane already."

>  - Even if we could solve it with "git add sub/" that adds to
>.gitmodules, is it a good solution, when we can solve the same
>thing without having to do so?

I am confused even more.

So you suggest that "git add [--gitlink=submodule]" taking on the
responsibilities of "git submodule add" is a bad idea?

I thought we had the same transition from "git remote update" to
"git fetch", which eventually superseded the former.

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Junio C Hamano]

Stefan Beller  writes:

> So you propose to make git-add behave like "git submodule add"
> (i.e. also add the .gitmodules entry for name/path/URL), which I
> like from a submodule perspective.
>
> However other users of gitlinks might be confused[1], which is why
> I refrained from "making every gitlink into a submodule". Specifically
> the more powerful a submodule operation is (the more fluff adds),
> the harder it should be for people to mis-use it.

A few questions that come to mind are:

 - Does "git add sub/" have enough information to populate
   .gitmodules?  If we have reasonable "default" values for
   .gitmodules entries (e.g. missing URL means we won't fetch when
   asked to go recursively fetch), perhaps we can leave everything
   other than "submodule.$name.path" undefined.

 - Can't we help those who have gitlinks without .gitmodules entries
   exactly the same way as above, i.e. when we see a gitlink and try
   to treat it as a submodule, we'd first try to look it up from
   .gitmodules (by going from path to name and then to
   submodule.$name.$var); the above "'git add sub/' would add an
   entry for .gitmodules" wish is based on the assumption that there
   are reasonable "default" values for each of these $var--so by
   basing on the same assumption, we can "pretend" as if these
   submodule.$name.$var were in .gitmodules file when we see
   gitlinks without .gitmodules entries.  IOW, if "git add sub/" can
   add .gitmodules to help people without having to type "git
   submodule add sub/", then we can give exactly the same degree of
   help without even modifying .gitmodules when "git add sub/" is
   run.

 - Even if we could solve it with "git add sub/" that adds to
   .gitmodules, is it a good solution, when we can solve the same
   thing without having to do so?

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Stefan Beller]

On Tue, Oct 10, 2017 at 6:03 AM, Heiko Voigt  wrote:

>> > As mentioned in the cover letter. This seems to be the only test that
>> > ensures that we stay compatible with setups without .gitmodules. Maybe
>> > we should add/revive some?
>>
>> An interesting discussion covering this topic is found at
>> https://public-inbox.org/git/20170606035650.oykbz2uc4xkr3...@sigill.intra.peff.net/
>
> Thanks for that pointer. So in that discussion Junio said that the
> recursive operations should succeed if we have everything necessary at
> hand. I kind of agree because why should we limit usage when not
> necessary. On the other hand we want git to be easy to use. And that
> example from Peff is perfect as a demonstration of a incosistency we
> currently have:
>
> git clone git://some.where.git/submodule.git
> git add submodule
>
> is an operation I remember, I did, when first getting in contact with
> submodules (many years back), since that is one intuitive way. And the
> thing is: It works, kind of... Only later I discovered that one actually
> needs to us a special submodule command to get everything approriately
> setup to work together with others.

I agree that we ought to not block off users "because we can", but rather
perform the operation if possible with the data at hand.

Note that the result of the discussion `jk/warn-add-gitlink actually`
warns about adding a raw gitlink now, such that we hint at using
"git submodule add", directly.

So you propose to make git-add behave like "git submodule add"
(i.e. also add the .gitmodules entry for name/path/URL), which I
like from a submodule perspective.

However other users of gitlinks might be confused[1], which is why
I refrained from "making every gitlink into a submodule". Specifically
the more powerful a submodule operation is (the more fluff adds),
the harder it should be for people to mis-use it.

[1] https://github.com/git-series/git-series/blob/master/INTERNALS.md
 "git-series uses gitlinks to store pointer to commits in its own repo."

> If everyone agrees that submodules are the default way of handling
> repositories insided repositories, IMO, 'git add' should also alter
> .gitmodules by default. We could provide a switch to avoid doing that.

I wonder if that switch should be default-on (i.e. not treat a gitlink as
a submodule initially, behavior as-is, and then eventually we will
die() on unconfigured repos, expecting the user to make the decision)

> An intermediate solution would be to warn

That is already implemented by Peff.

> but in the long run my goal
> for submodules is and always was: Make them behave as close to files as
> possible. And why should a 'git add submodule' not magically do
> everything it can to make submodules just work? I can look into a patch
> for that if people agree here...

I'd love to see this implemented. I cc'd Josh (the author of git-series), who
may disagree with this, or has some good input how to go forward without
breaking git-series.

> Regarding handling of gitlinks with or without .gitmodules:
>
> Currently we are actually in some intermediate state:
>
>  * If there is no .gitmodules file: No submodule processing on any
>gitlinks (AFAIK)

AFAIK this is true.

>  * If there is a .gitmodules files with some submodule configured: Do
>recursive fetch and push as far as possible on gitlinks.

* If submodule.recurse is set, then we also treat submodules like files
  for checkout, reset, read-tree.

> So I am not sure whether there are actually many users (knowingly)
> using a mix of some submodules configured and some not and then relying
> on the submodule infrastructure.
>
> I would rather expect two sorts of users:
>
>   1. Those that do use .gitmodules

Those want to reap all benefits of good submodules.

>
>   2. Those that do *not* use .gitmodules

As said above, we don't know if those users are
"holding submodules wrong" or are using gitlinks for
magic tricks (unrelated to submodules).

>
> Users that do not use any .gitmodules file will currently (AFAIK) not
> get any submodule handling. So the question is are there really many
> "mixed users"? My guess would be no.

I hope that there are few (if any) users of these mixed setups.

> Because without those using this mixed we could switch to saying: "You
> need to have a .gitmodules file for submodule handling" without much
> fallout from breaking users use cases.

That seems reasonable to me, actually.

> Maybe we can test this out somehow? My patch series would be ready in
> that case, just had to drop the first patch and adjust the commit
> message of this one.

I wonder how we would test this, though? Do you have any idea
(even vague) how we'd accomplish such a measurement?
I fear we'll have to go this way blindly.

Cheers,
Stefan

>
> Cheers Heiko

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Heiko Voigt]

Hi,

On Mon, Oct 09, 2017 at 11:20:51AM -0700, Stefan Beller wrote:
> On Fri, Oct 6, 2017 at 3:32 PM, Heiko Voigt  wrote:
> > NOTE: The argument in this message is not correct, see description in
> > cover letter.
> >
> > The setup of the repositories in this test is using gitlinks without the
> > .gitmodules infrastructure. It is however testing convenience features
> > like --recurse-submodules=on-demand. These features are already not
> > supported by fetch without a .gitmodules file. This leads us to the
> > conclusion that it is not really used here as well.
> >
> > Let's use the usual submodule commands to setup the repository in a
> > typical way. This also has the advantage that we are testing with a
> > repository structure that is more similar to one we could expect on a
> > users setup.
> >
> > Signed-off-by: Heiko Voigt 
> > ---
> >
> > As mentioned in the cover letter. This seems to be the only test that
> > ensures that we stay compatible with setups without .gitmodules. Maybe
> > we should add/revive some?
> 
> An interesting discussion covering this topic is found at
> https://public-inbox.org/git/20170606035650.oykbz2uc4xkr3...@sigill.intra.peff.net/

Thanks for that pointer. So in that discussion Junio said that the
recursive operations should succeed if we have everything necessary at
hand. I kind of agree because why should we limit usage when not
necessary. On the other hand we want git to be easy to use. And that
example from Peff is perfect as a demonstration of a incosistency we
currently have:

git clone git://some.where.git/submodule.git
git add submodule

is an operation I remember, I did, when first getting in contact with
submodules (many years back), since that is one intuitive way. And the
thing is: It works, kind of... Only later I discovered that one actually
needs to us a special submodule command to get everything approriately
setup to work together with others.

If everyone agrees that submodules are the default way of handling
repositories insided repositories, IMO, 'git add' should also alter
.gitmodules by default. We could provide a switch to avoid doing that.

An intermediate solution would be to warn but in the long run my goal
for submodules is and always was: Make them behave as close to files as
possible. And why should a 'git add submodule' not magically do
everything it can to make submodules just work? I can look into a patch
for that if people agree here...

Regarding handling of gitlinks with or without .gitmodules:

Currently we are actually in some intermediate state:

 * If there is no .gitmodules file: No submodule processing on any
   gitlinks (AFAIK)
 * If there is a .gitmodules files with some submodule configured: Do
   recursive fetch and push as far as possible on gitlinks.

So I am not sure whether there are actually many users (knowingly)
using a mix of some submodules configured and some not and then relying
on the submodule infrastructure.

I would rather expect two sorts of users:

  1. Those that do use .gitmodules

  2. Those that do *not* use .gitmodules

Users that do not use any .gitmodules file will currently (AFAIK) not
get any submodule handling. So the question is are there really many
"mixed users"? My guess would be no.
Because without those using this mixed we could switch to saying: "You
need to have a .gitmodules file for submodule handling" without much
fallout from breaking users use cases.

Maybe we can test this out somehow? My patch series would be ready in
that case, just had to drop the first patch and adjust the commit
message of this one.

Cheers Heiko

Re: [RFC PATCH 2/4] change submodule push test to use proper repository setup

[Stefan Beller]

On Fri, Oct 6, 2017 at 3:32 PM, Heiko Voigt  wrote:
> NOTE: The argument in this message is not correct, see description in
> cover letter.
>
> The setup of the repositories in this test is using gitlinks without the
> .gitmodules infrastructure. It is however testing convenience features
> like --recurse-submodules=on-demand. These features are already not
> supported by fetch without a .gitmodules file. This leads us to the
> conclusion that it is not really used here as well.
>
> Let's use the usual submodule commands to setup the repository in a
> typical way. This also has the advantage that we are testing with a
> repository structure that is more similar to one we could expect on a
> users setup.
>
> Signed-off-by: Heiko Voigt 
> ---
>
> As mentioned in the cover letter. This seems to be the only test that
> ensures that we stay compatible with setups without .gitmodules. Maybe
> we should add/revive some?

An interesting discussion covering this topic is found at
https://public-inbox.org/git/20170606035650.oykbz2uc4xkr3...@sigill.intra.peff.net/