Message ID | a8125b946227d918865fde0dcbec516474b42386.1574731649.git.jonathantanmy@google.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Advice upon clone --recurse-submodules --reference | expand |
On Mon, Nov 25, 2019 at 05:30:59PM -0800, Jonathan Tan wrote: > Commit 31224cbdc7 ("clone: recursive and reference option triggers > submodule alternates", 2016-08-17) taught Git to support the > configuration options "submodule.alternateLocation" and > "submodule.alternateErrorStrategy" on a superproject. > > If "submodule.alternateLocation" is configured to "superproject" on a > superproject, whenever a submodule of that superproject is cloned, it > instead computes the analogous alternate path for that submodule from > $GIT_DIR/objects/info/alternates of the superproject, and references it. > > The "submodule.alternateErrorStrategy" option determines what happens > if that alternate cannot be referenced. However, it is not clear that > the clone proceeds as if no alternate was specified when that option is > not set to "die" (as can be seen in the tests in 31224cbdc7). Therefore, > document it accordingly. > > Signed-off-by: Jonathan Tan <jonathantanmy@google.com> > --- > When I said "not clear" above, I mean that it is not clear *to me*, and > I assume that others will feel the same way. Feel free to drop this from > the patch set if the existing documentation is clear to most people. FWIW, I learned about these options for the first time from your email, and I think the suggested change makes the behavior much more clear. -Peff
Jonathan Tan <jonathantanmy@google.com> writes: > Commit 31224cbdc7 ("clone: recursive and reference option triggers > submodule alternates", 2016-08-17) taught Git to support the > configuration options "submodule.alternateLocation" and > "submodule.alternateErrorStrategy" on a superproject. > ... > The "submodule.alternateErrorStrategy" option determines what happens > if that alternate cannot be referenced. However, it is not clear that > the clone proceeds as if no alternate was specified when that option is > not set to "die" (as can be seen in the tests in 31224cbdc7). Therefore, > document it accordingly. Given that for everyday use (cf. sha1-file.c::link_alt_odb_entry()) of an alternate is best-effort basis, I have a feeling that it was a design mistake to have the "error strategy" configuration option in the first place, and "clone --reference-if-able" was the result of the same design mistake. We would have been better off if we made these the best-effort features as well. But the ship has sailed long ago---so I think these two are the best we can do at this point. Perhaps flipping the default to warn may be a longer term improvement, too.
diff --git a/Documentation/config/submodule.txt b/Documentation/config/submodule.txt index 0a1293b051..b33177151c 100644 --- a/Documentation/config/submodule.txt +++ b/Documentation/config/submodule.txt @@ -79,4 +79,6 @@ submodule.alternateLocation:: submodule.alternateErrorStrategy:: Specifies how to treat errors with the alternates for a submodule as computed via `submodule.alternateLocation`. Possible values are - `ignore`, `info`, `die`. Default is `die`. + `ignore`, `info`, `die`. Default is `die`. Note that if set to `ignore` + or `info`, and if there is an error with the computed alternate, the + clone proceeds as if no alternate was specified.
Commit 31224cbdc7 ("clone: recursive and reference option triggers submodule alternates", 2016-08-17) taught Git to support the configuration options "submodule.alternateLocation" and "submodule.alternateErrorStrategy" on a superproject. If "submodule.alternateLocation" is configured to "superproject" on a superproject, whenever a submodule of that superproject is cloned, it instead computes the analogous alternate path for that submodule from $GIT_DIR/objects/info/alternates of the superproject, and references it. The "submodule.alternateErrorStrategy" option determines what happens if that alternate cannot be referenced. However, it is not clear that the clone proceeds as if no alternate was specified when that option is not set to "die" (as can be seen in the tests in 31224cbdc7). Therefore, document it accordingly. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- When I said "not clear" above, I mean that it is not clear *to me*, and I assume that others will feel the same way. Feel free to drop this from the patch set if the existing documentation is clear to most people. --- Documentation/config/submodule.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)