Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-10 Thread Junio C Hamano
Stefan Beller  writes:

> On Tue, Aug 9, 2016 at 2:45 PM, Junio C Hamano  wrote:
>> becomes easily doable (i.e. subsequent "submodule update" can realize
>> that the submodule does not have alternates but it could borrow from
>> the submodule in the other-super-project-location).
>
> I would suggest to postpone this to a later time once the need arises.

Oh, no question about that (did I even sound to be suggesting to do
it now?).  It is just one of the yardsticks I use when gauging if a
proposed solution is sound to see how it will naturally scale and/or
extend to other possible scenarios, and I was pointing out that this
seems to pass the test.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-09 Thread Stefan Beller
On Tue, Aug 9, 2016 at 2:45 PM, Junio C Hamano  wrote:
> becomes easily doable (i.e. subsequent "submodule update" can realize
> that the submodule does not have alternates but it could borrow from
> the submodule in the other-super-project-location).

I would suggest to postpone this to a later time once the need arises.

I rather imagine that once you clone from "other-super-project-location"
and get the warning about no alternates borrowing, you can decide if you want
to set it up and link the submodule manually to that alternate, or if you just
don't care about this one repository being duplicated.

For now I plan to introduce 2 options, as these are 2 different things, that
can be extended independently of each other:

submodule.alternateLocation::
Specifies how the submodules obtain alternates when submodules are
cloned. Possible values are `no`, `superproject`.
By default `no` is assumed, which doesn't add references. When the
value is set to `superproject` the submodule to be cloned computes
its alternates location relative to the superprojects alternate.

submodule.alternateErrorStrategy::
Specifies how to treat errors with the alternates for a submodule
as computed via `submodule.alternateLocation`. Possible values are
`ignore`, `info`, `die`.

> *1* Rather, I meant: clone has a very intimate knowledge on what and
> what cannot be borrowed from and it is not just "is there a
> directory?", so "git submodule update --init" is not in a good
> position to decide if it wants to add --reference to the
> invocation of "git clone" it makes internally, and introducing
> an "if-able" variant to "clone" is one way to relieve it from
> having to make that decision.

That is how I first understood the design as well.

>
> I could have suggested an alternative: because the submodule
> machinery is gettting moved to C the "update --init" code that
> drives the internal invocation of "git clone" could share the
> the logic in "git clone --reference" that determines if a local
> repository can be used as an alternate by small refactoring of
> builtin/clone.c::add_one_reference().

I see. I might actually need to do this anyway as the helper is a place
to act on the submodule.alternateErrorStrategy.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-09 Thread Junio C Hamano
Stefan Beller  writes:

> At the time of cloning you may run
>
>   git clone --recursive --reference 
> or
>   git clone --recursive --reference-if-able 
> or
>   git clone --recursive

That's an interesting tangent.  I never meant "if-able" to be an
end-user visible option [*1*], but now you mention it, I do not see
a reason why "clone --reference-if-able" of the top-level project
cannot be used together with "--recursive".

> Then later when we run
> git submodule update
> we have this option to know if a submodule alternate should be treated
> as optional or required referenced as the existence
> of the superprojects alternate (as a boolean indicator) is not enough of
> an indicator what the user later wants.

A tangent that comes to my mind after reading this is if letting
"if-able" just skip (with or without warning) and forget about it
once a clone is made is what we really want.

Suppose the "other-super-project-location" repository did not have a
clone of a submodule when you create a new clone of the superproject
using it as a reference.  The submodule will be made a full clone,
but after that happens, other-super-project-location can get
interested in the submodule and can acquire its own clone.

At that point, our clone of the submodule _could_ add the submodule
in the other-super-project-location as its alternate and lose the
duplicate objects that it could borrow from there by repacking, but
the suggested "clone with if-able, and forget about it after a clone
is made" would not allow us to do this.  I do not know if a
real-world use of submodules want the ability to do so, or it is
unnecessary.  I suspect with the recording of "you were told that
borrowing from the same location as the superproject is OK", this
becomes easily doable (i.e. subsequent "submodule update" can realize
that the submodule does not have alternates but it could borrow from
the submodule in the other-super-project-location).


[Footnote]

*1* Rather, I meant: clone has a very intimate knowledge on what and
what cannot be borrowed from and it is not just "is there a
directory?", so "git submodule update --init" is not in a good
position to decide if it wants to add --reference to the
invocation of "git clone" it makes internally, and introducing
an "if-able" variant to "clone" is one way to relieve it from
having to make that decision.

I could have suggested an alternative: because the submodule
machinery is gettting moved to C the "update --init" code that
drives the internal invocation of "git clone" could share the
the logic in "git clone --reference" that determines if a local
repository can be used as an alternate by small refactoring of
builtin/clone.c::add_one_reference().
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-09 Thread Stefan Beller
On Tue, Aug 9, 2016 at 11:44 AM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> The way I understood and implemented it is
>>
>> here is a path, try to use it as an alternate; if that is not
>> an alternate, it's fine too; maybe warn about it, but carry
>> on with the operation.
>
> My expectation is without "maybe warn about it".  And not adding it
> as an alternate (because it is not usable) is just "doing as I was
> told to do", nothing noteworthy.  Because "do it only if you can" is
> an explicit instruction to the doer that the caller does not care
> about the outcome, I'd think there shouldn't be any warning, whether
> it is used or not.  At the same time, if the caller wants to know
> the outcome, and using if-able as a way to say "I prefer to see it
> happen, but you do not have to panic if you can't", I'd think it is
> OK to give "info:" to report which of the two possible paths was
> taken in either case.  Throwing a "warning:" only when it didn't do
> so does not make much sense to me.

I think this whole thing needs a new config variable to be set.

At the time of cloning you may run

  git clone --recursive --reference 
or
  git clone --recursive --reference-if-able 
or
  git clone --recursive

The first two should trickle down to the submodules, i.e.
in the first case we maybe want to run

git config submodule.alternate=required-superproject &&
git submodule update --init --recurse

In the second case
git config submodule.alternate=optional-superproject &&
git submodule update --init --recurse

And in the third case we maybe only want to record
git config submodule.alternate=none # implicit by not setting it as well
git submodule update --init --recurse


Then later when we run
git submodule update
we have this option to know if a submodule alternate should be treated
as optional or required referenced as the existence
of the superprojects alternate (as a boolean indicator) is not enough of
an indicator what the user later wants.


>>
>> I see. This way the user is most informed.
>
> Yes, and if we were to go that route, "clone" without "-v" should
> not report which of the two it took.  It is OK for "submodule" to
> look at what "clone" left as the result and do more intelligent
> reporting that is better suited for the context of the command.
>
>> The current implementation
>> of "submodule update --init" for start from scratch yields for example:
>>
>> Submodule 'foo' () registered for path 'foo'
>> Submodule 'hooks' () registered for path 'hooks'
>> Cloning into '/home/sbeller/super/foo'...
>> Cloning into '/home/sbeller/super/hooks'...
>> warning: Not using proposed alternate
>> /home/sbeller/super-reference/.git/modules/hooks/
>> Submodule path 'foo': checked out '7b41f3a413b46140b050ae5324cbbcdd467d2b3a'
>> Submodule path 'hooks': checked out 
>> '3acc14d10d26678eae6489038fe0d4dad644a9b4'
>>
>> so before this series we had 3 lines per submodule, and with this series
>> we get a 4th line for the unusual case.
>>
>> You would prefer to see always 4 lines per submodule?
>
> If the user says "--recursive --reference", then the user probably
> deserves to be notified.  As I said (eh, rather, as I wanted to say
> but failed to say so), I do not have a strong preference either way.

With a new config option we can trigger the notifications based on that as well.

"required superproject" would die when the submodule alternate is not there,
and the "optional superproject" would always state if it found an alternate.

Thanks,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-09 Thread Junio C Hamano
Stefan Beller  writes:

> The way I understood and implemented it is
>
> here is a path, try to use it as an alternate; if that is not
> an alternate, it's fine too; maybe warn about it, but carry
> on with the operation.

My expectation is without "maybe warn about it".  And not adding it
as an alternate (because it is not usable) is just "doing as I was
told to do", nothing noteworthy.  Because "do it only if you can" is
an explicit instruction to the doer that the caller does not care
about the outcome, I'd think there shouldn't be any warning, whether
it is used or not.  At the same time, if the caller wants to know
the outcome, and using if-able as a way to say "I prefer to see it
happen, but you do not have to panic if you can't", I'd think it is
OK to give "info:" to report which of the two possible paths was
taken in either case.  Throwing a "warning:" only when it didn't do
so does not make much sense to me.

> The way you write this response I think you have a slightly
> different understanding of what the -if-able ought to do?
>
> When --reference is given, only the superproject should
> borrow and the -if-able, may allow submodules to also borrow?

I have no idea what this sentence means.

>> I have a very strong opinion what should happen when the end-user
>> facing command is "git submodule", but if I have to choose, I would
>> prefer to see "git submodule update" tell what it did with "info:"
>> either case, i.e. "info: borrowing from ... because the superproject
>> borrows from there", and "info: not borrowing from ... even though
>> the superproject borrows from there".
>
> I see. This way the user is most informed.

Yes, and if we were to go that route, "clone" without "-v" should
not report which of the two it took.  It is OK for "submodule" to
look at what "clone" left as the result and do more intelligent
reporting that is better suited for the context of the command.

> The current implementation
> of "submodule update --init" for start from scratch yields for example:
>
> Submodule 'foo' () registered for path 'foo'
> Submodule 'hooks' () registered for path 'hooks'
> Cloning into '/home/sbeller/super/foo'...
> Cloning into '/home/sbeller/super/hooks'...
> warning: Not using proposed alternate
> /home/sbeller/super-reference/.git/modules/hooks/
> Submodule path 'foo': checked out '7b41f3a413b46140b050ae5324cbbcdd467d2b3a'
> Submodule path 'hooks': checked out '3acc14d10d26678eae6489038fe0d4dad644a9b4'
>
> so before this series we had 3 lines per submodule, and with this series
> we get a 4th line for the unusual case.
>
> You would prefer to see always 4 lines per submodule?

If the user says "--recursive --reference", then the user probably
deserves to be notified.  As I said (eh, rather, as I wanted to say
but failed to say so), I do not have a strong preference either way.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-09 Thread Stefan Beller
On Tue, Aug 9, 2016 at 10:47 AM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> "you did ask me to use alternates once and for all when setting up the
>> superproject: now for this added submodule I don't find the alternate;
>> That is strange?"
>
> Absolutely.  I do not think you should expect a user to remember if
> s/he used alternates when getting a copy of the superproject long
> time ago.  Giving "info: using from ..." is good; giving "warning:
> not using from ..." is probably irritating, I would think.
>
> Stepping back a bit, when the user says --reference-if-able, does it
> warrant any warning either way?

Stepping another step back, let's discuss the expectations of the
new option "--reference-if-able".

The way I understood and implemented it is

here is a path, try to use it as an alternate; if that is not
an alternate, it's fine too; maybe warn about it, but carry
on with the operation.

With such a semantics you can just add the --reference-if-able
to the submodules that try to borrow from the other superprojects
submodule.

I expected the "--reference-if-able" not necessarily be
used by the end user. It is a convenient way for our scripted
use, as the -if-able is just a check if the path exists and nothing
else.

We could check if the git dir exists inthe helper for the
submodule command, such that we only pass --reference
as we are certain the alternate exists; we could have a switch
in the helper --on-missing-submodule-alternate=[die,info,silent]

The way you write this response I think you have a slightly
different understanding of what the -if-able ought to do?

When --reference is given, only the superproject should
borrow and the -if-able, may allow submodules to also borrow?

> I.e. "we made it borrow from there,
> so be careful not to trash that one" may be just as warning-worthy
> (if not even more) as "you said we can borrow from there if there is
> anything to borrow, but it turns out there isn't any, so the result
> is complete stand-alone."  It feels as if we can go without any
> warning at least from "git clone --reference-if-able", unless "-v"
> option is given.

But when git clone is not issueing a warning/info, who is responsible for
that? As you noted the superproject may be setup some time ago and
the user forgot they used references and want to use references for this
new submodule. So the helper would need to do that?

>
> I have a very strong opinion what should happen when the end-user
> facing command is "git submodule", but if I have to choose, I would
> prefer to see "git submodule update" tell what it did with "info:"
> either case, i.e. "info: borrowing from ... because the superproject
> borrows from there", and "info: not borrowing from ... even though
> the superproject borrows from there".

I see. This way the user is most informed. The current implementation
of "submodule update --init" for start from scratch yields for example:

Submodule 'foo' () registered for path 'foo'
Submodule 'hooks' () registered for path 'hooks'
Cloning into '/home/sbeller/super/foo'...
Cloning into '/home/sbeller/super/hooks'...
warning: Not using proposed alternate
/home/sbeller/super-reference/.git/modules/hooks/
Submodule path 'foo': checked out '7b41f3a413b46140b050ae5324cbbcdd467d2b3a'
Submodule path 'hooks': checked out '3acc14d10d26678eae6489038fe0d4dad644a9b4'

so before this series we had 3 lines per submodule, and with this series
we get a 4th line for the unusual case.

You would prefer to see always 4 lines per submodule?
Is one extra line (25% more output) a reasonable tradeoff for the
reference feature?

I dunno; I guess we could argue either way.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-09 Thread Junio C Hamano
Junio C Hamano  writes:

> Stefan Beller  writes:
>
>> "you did ask me to use alternates once and for all when setting up the
>> superproject: now for this added submodule I don't find the alternate;
>> That is strange?"
>
> Absolutely.  I do not think you should expect a user to remember if
> s/he used alternates when getting a copy of the superproject long
> time ago.  Giving "info: using from ..." is good; giving "warning:
> not using from ..." is probably irritating, I would think.
>
> Stepping back a bit, when the user says --reference-if-able, does it
> warrant any warning either way?  I.e. "we made it borrow from there,
> so be careful not to trash that one" may be just as warning-worthy
> (if not even more) as "you said we can borrow from there if there is
> anything to borrow, but it turns out there isn't any, so the result
> is complete stand-alone."  It feels as if we can go without any
> warning at least from "git clone --reference-if-able", unless "-v"
> option is given.
>
> I have a very strong opinion what should happen when the end-user

Too many proof-reading and rephrasing.  "I DON'T have a very strong
opinion" is what I wanted to say.  Sorry for the lack of last-time
proof-reading.

> facing command is "git submodule", but if I have to choose, I would
> prefer to see "git submodule update" tell what it did with "info:"
> either case, i.e. "info: borrowing from ... because the superproject
> borrows from there", and "info: not borrowing from ... even though
> the superproject borrows from there".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-09 Thread Junio C Hamano
Stefan Beller  writes:

> "you did ask me to use alternates once and for all when setting up the
> superproject: now for this added submodule I don't find the alternate;
> That is strange?"

Absolutely.  I do not think you should expect a user to remember if
s/he used alternates when getting a copy of the superproject long
time ago.  Giving "info: using from ..." is good; giving "warning:
not using from ..." is probably irritating, I would think.

Stepping back a bit, when the user says --reference-if-able, does it
warrant any warning either way?  I.e. "we made it borrow from there,
so be careful not to trash that one" may be just as warning-worthy
(if not even more) as "you said we can borrow from there if there is
anything to borrow, but it turns out there isn't any, so the result
is complete stand-alone."  It feels as if we can go without any
warning at least from "git clone --reference-if-able", unless "-v"
option is given.

I have a very strong opinion what should happen when the end-user
facing command is "git submodule", but if I have to choose, I would
prefer to see "git submodule update" tell what it did with "info:"
either case, i.e. "info: borrowing from ... because the superproject
borrows from there", and "info: not borrowing from ... even though
the superproject borrows from there".


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-09 Thread Stefan Beller
On Tue, Aug 9, 2016 at 8:49 AM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> v3:
>>
>> Thanks to Junios critial questions regarding the design, I took a step back
>> to look at the bigger picture.
>>
>> --super-reference sounds confusing. (what is the super referring to?)
>> So drop that approach.
>>
>> Instead we'll compute where the reference might be in the superproject scope
>> and ask the submodule clone operation to consider an optional reference.
>> If the referenced alternate is not there, we'll just warn about it and
>> carry on.
>
> I like the general direction, but I hope that the warning comes only
> when the user said "--reference" on the command line (i.e. "you
> asked me to use reference, but for this submodule I couldn't find a
> usable one").

No it comes all the time now, as there is no difference between
"clone --recursive" and later running "submodule update" manually.

Assume a new submodule is added and you forget to update your
main alternate all your other clones are borrowing from. If there is
no warning you create a real clone for the new submodule all the time,
which is what you wanted to avoid in the first place?

>  If the implementation allows the same mechanism to
> help later "submodule init && submodule update" borrow from the
> submodule repositories of the superproject the current superproject
> borrows from (i.e. no explicit "--reference" on the command line
> when doing init/update), I would think the case that needs warning
> is "you didn't explicitly ask me to borrow objects, but I found one
> we could, so I did it anyway without being asked", and it is not a
> warning-worthy condition if we didn't find a cloned submodule in the
> repository our superproject borrows from.

"you did ask me to use alternates once and for all when setting up the
superproject: now for this added submodule I don't find the alternate;
That is strange?"

Thanks,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-09 Thread Junio C Hamano
Stefan Beller  writes:

> v3:
>
> Thanks to Junios critial questions regarding the design, I took a step back
> to look at the bigger picture. 
>
> --super-reference sounds confusing. (what is the super referring to?)
> So drop that approach.
>
> Instead we'll compute where the reference might be in the superproject scope
> and ask the submodule clone operation to consider an optional reference.
> If the referenced alternate is not there, we'll just warn about it and
> carry on.

I like the general direction, but I hope that the warning comes only
when the user said "--reference" on the command line (i.e. "you
asked me to use reference, but for this submodule I couldn't find a
usable one").  If the implementation allows the same mechanism to
help later "submodule init && submodule update" borrow from the
submodule repositories of the superproject the current superproject
borrows from (i.e. no explicit "--reference" on the command line
when doing init/update), I would think the case that needs warning
is "you didn't explicitly ask me to borrow objects, but I found one
we could, so I did it anyway without being asked", and it is not a
warning-worthy condition if we didn't find a cloned submodule in the
repository our superproject borrows from.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-08 Thread Jacob Keller
On Mon, Aug 8, 2016 at 9:08 PM, Stefan Beller  wrote:
>
> v3:
>
> Thanks to Junios critial questions regarding the design, I took a step back
> to look at the bigger picture.
>
> --super-reference sounds confusing. (what is the super referring to?)
> So drop that approach.
>
> Instead we'll compute where the reference might be in the superproject scope
> and ask the submodule clone operation to consider an optional reference.
> If the referenced alternate is not there, we'll just warn about it and
> carry on.
>

This makes a lot more sense to me, and being able to use
"reference-if-able" for regular clones may be a useful addition as
well.

I looked over the other patches and didn't see anything obviously
wrong, but take that with a grain of salt as I didn't spend a lot of
time at it.

Thanks,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-08 Thread Stefan Beller

v3:

Thanks to Junios critial questions regarding the design, I took a step back
to look at the bigger picture. 

--super-reference sounds confusing. (what is the super referring to?)
So drop that approach.

Instead we'll compute where the reference might be in the superproject scope
and ask the submodule clone operation to consider an optional reference.
If the referenced alternate is not there, we'll just warn about it and
carry on.



* fixed the style in patch 2.

* fixed another bug in the last patch, that is unrelated, but would have helped
  me a lot.

Thanks,
Stefan

 Documentation/git-clone.txt|   9 -
 builtin/clone.c|  36 
 builtin/submodule--helper.c| 105 
-
 git-submodule.sh   |   2 +-
 t/t7408-submodule-reference.sh | 162 
--
 5 files changed, 217 insertions(+), 97 deletions(-)

v2:
 * fixed the p1,2 cleanups
 * added documentation to patches 5,6
 * improved commit message in v4
 
 Thanks,
 Stefan
 
v1:
 
 Currently when cloning a superproject with --recursive and --reference
 only the superproject learns about its alternates. The submodules are
 cloned independently, which may incur lots of network costs.
 
 Assume that the reference repository has the submodules at the same
 paths as the to-be-cloned submodule and try to setup alternates from
 there.
 
 Some submodules in the referenced superproject may not be there, 
 (they are just not initialized/cloned/checked out), which yields
 an error for now. In future work we may want to soften the alternate
 check and not die in the clone when one of the given alternates doesn't
 exist.
 
 patch 1,2 are modernizing style of t7408, 
 patches 3,4 are not strictly necessary, but I think it is a good thing
 to not leave the submodule related C code in a crippled state (i.e.
 allowing only one reference). The shell code would also need this update,
 but it looked ugly to me, so I postpone it until more of the submodule code
 is written in C. 
 
 Thanks,
 Stefan 

Stefan Beller (6):
  t7408: modernize style
  t7408: merge short tests, factor out testing method
  submodule--helper module-clone: allow multiple references
  submodule--helper update-clone: allow multiple references
  submodule update: add super-reference flag
  clone: reference flag is used for submodules as well

 builtin/clone.c|  22 --
 builtin/submodule--helper.c|  45 
 git-submodule.sh   |  12 +++-
 t/t7408-submodule-reference.sh | 153 +++--
 4 files changed, 147 insertions(+), 85 deletions(-)

-- 
2.9.2.572.g9d9644e.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-08 Thread Stefan Beller
On Sat, Aug 6, 2016 at 10:29 AM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>>  Some submodules in the referenced superproject may not be there,
>>  (they are just not initialized/cloned/checked out), which yields
>>  an error for now.
>
> Perhaps you can teach "git clone --reference" an new option
> (--reference-if-able) to do this?  Then
>
> When `--reference` is given together with `--recursive`,
> the reference repository is assumed to contain the submodules
> as well and the submodules are setup as alternates of the
> submodules in the given reference project.
>
> in which "assumed" is a horrible wording (leave the reader
> wondering: "so what happens to my data when the assumption does not
> hold") can become a lot more reasonable
>
> When using --reference with --recursive, the --reference is used
> to specify a repository that has a copy of the superproject.  If
> that copy has submodules cloned for itself in its $GIT_DIR/modules,
> they are used as --reference when cloning submodules in the
> resulting clone.
>
> and readers expectation would match with the reality.  Their
> submodules would be cloned in a regular fashion if the central
> mirror does not have it, and would take advantage of it if there is
> already a clone.

That makes sense.

>
> Come to think of it, do we even need --super-reference?  "git clone
> --reference --recursive" is a two step process, in that first the
> superproject is cloned while creating objects/info/alternates, and
> then submodules are cloned (via "update --init").  Can we make the
> procedure to clone a submodule always look at the reference of the
> superproject (i.e. objects/info/alternates) and try to borrow from
> the place in it that corresponds to the submodule?  That way, not
> just "git clone --reference --recursive" would take advantage of the
> existing mirrors of submodules, a user who does this:
>
> $ git clone --reference $URL super
> $ cd super
> $ git submodule update --init ...
>
> would be able to take advantage of the "what the mirror the
> superproject uses already has" when cloning the submodules, no?

That would work, too.

I'll implement that.

Thanks,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-06 Thread Stefan Beller
v2:
 * fixed the p1,2 cleanups
 * added documentation to patches 5,6
 * improved commit message in v4
 
 Thanks,
 Stefan
 
v1:
 
 Currently when cloning a superproject with --recursive and --reference
 only the superproject learns about its alternates. The submodules are
 cloned independently, which may incur lots of network costs.
 
 Assume that the reference repository has the submodules at the same
 paths as the to-be-cloned submodule and try to setup alternates from
 there.
 
 Some submodules in the referenced superproject may not be there, 
 (they are just not initialized/cloned/checked out), which yields
 an error for now. In future work we may want to soften the alternate
 check and not die in the clone when one of the given alternates doesn't
 exist.
 
 patch 1,2 are modernizing style of t7408, 
 patches 3,4 are not strictly necessary, but I think it is a good thing
 to not leave the submodule related C code in a crippled state (i.e.
 allowing only one reference). The shell code would also need this update,
 but it looked ugly to me, so I postpone it until more of the submodule code
 is written in C. 
 
 Thanks,
 Stefan 

Stefan Beller (6):
  t7408: modernize style
  t7408: merge short tests, factor out testing method
  submodule--helper module-clone: allow multiple references
  submodule--helper update-clone: allow multiple references
  submodule update: add super-reference flag
  clone: reference flag is used for submodules as well

 builtin/clone.c|  22 --
 builtin/submodule--helper.c|  45 
 git-submodule.sh   |  12 +++-
 t/t7408-submodule-reference.sh | 153 +++--
 4 files changed, 147 insertions(+), 85 deletions(-)

-- 
2.9.2.572.g9d9644e.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference

2016-08-06 Thread Junio C Hamano
Stefan Beller  writes:

>  Some submodules in the referenced superproject may not be there, 
>  (they are just not initialized/cloned/checked out), which yields
>  an error for now.

Perhaps you can teach "git clone --reference" an new option
(--reference-if-able) to do this?  Then 

When `--reference` is given together with `--recursive`,
the reference repository is assumed to contain the submodules
as well and the submodules are setup as alternates of the
submodules in the given reference project.
 
in which "assumed" is a horrible wording (leave the reader
wondering: "so what happens to my data when the assumption does not
hold") can become a lot more reasonable

When using --reference with --recursive, the --reference is used
to specify a repository that has a copy of the superproject.  If
that copy has submodules cloned for itself in its $GIT_DIR/modules,
they are used as --reference when cloning submodules in the
resulting clone.

and readers expectation would match with the reality.  Their
submodules would be cloned in a regular fashion if the central
mirror does not have it, and would take advantage of it if there is
already a clone.

Come to think of it, do we even need --super-reference?  "git clone
--reference --recursive" is a two step process, in that first the
superproject is cloned while creating objects/info/alternates, and
then submodules are cloned (via "update --init").  Can we make the
procedure to clone a submodule always look at the reference of the
superproject (i.e. objects/info/alternates) and try to borrow from
the place in it that corresponds to the submodule?  That way, not
just "git clone --reference --recursive" would take advantage of the
existing mirrors of submodules, a user who does this:

$ git clone --reference $URL super
$ cd super
$ git submodule update --init ...

would be able to take advantage of the "what the mirror the
superproject uses already has" when cloning the submodules, no?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html