* [PATCH v6 1/4] lib-proto-disable: variable name fix
From: Brandon Williams @ 2016-12-01 19:44 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, peff, sbeller, bburky, jrnieder
In-Reply-To: <1480621447-52399-1-git-send-email-bmwill@google.com>
The test_proto function assigns the positional parameters to named
variables, but then still refers to "$desc" as "$1". Using $desc is
more readable and less error-prone.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
t/lib-proto-disable.sh | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/lib-proto-disable.sh b/t/lib-proto-disable.sh
index b0917d9..be88e9a 100644
--- a/t/lib-proto-disable.sh
+++ b/t/lib-proto-disable.sh
@@ -9,7 +9,7 @@ test_proto () {
proto=$2
url=$3
- test_expect_success "clone $1 (enabled)" '
+ test_expect_success "clone $desc (enabled)" '
rm -rf tmp.git &&
(
GIT_ALLOW_PROTOCOL=$proto &&
@@ -18,7 +18,7 @@ test_proto () {
)
'
- test_expect_success "fetch $1 (enabled)" '
+ test_expect_success "fetch $desc (enabled)" '
(
cd tmp.git &&
GIT_ALLOW_PROTOCOL=$proto &&
@@ -27,7 +27,7 @@ test_proto () {
)
'
- test_expect_success "push $1 (enabled)" '
+ test_expect_success "push $desc (enabled)" '
(
cd tmp.git &&
GIT_ALLOW_PROTOCOL=$proto &&
@@ -36,7 +36,7 @@ test_proto () {
)
'
- test_expect_success "push $1 (disabled)" '
+ test_expect_success "push $desc (disabled)" '
(
cd tmp.git &&
GIT_ALLOW_PROTOCOL=none &&
@@ -45,7 +45,7 @@ test_proto () {
)
'
- test_expect_success "fetch $1 (disabled)" '
+ test_expect_success "fetch $desc (disabled)" '
(
cd tmp.git &&
GIT_ALLOW_PROTOCOL=none &&
@@ -54,7 +54,7 @@ test_proto () {
)
'
- test_expect_success "clone $1 (disabled)" '
+ test_expect_success "clone $desc (disabled)" '
rm -rf tmp.git &&
(
GIT_ALLOW_PROTOCOL=none &&
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v6 3/4] http: always warn if libcurl version is too old
From: Brandon Williams @ 2016-12-01 19:44 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, peff, sbeller, bburky, jrnieder
In-Reply-To: <1480621447-52399-1-git-send-email-bmwill@google.com>
Now that there are default "known-good" and "known-bad" protocols which
are allowed/disallowed by 'is_transport_allowed' we should always warn
the user that older versions of libcurl can't respect the allowed
protocols for redirects.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
http.c | 5 ++---
transport.c | 5 -----
transport.h | 6 ------
3 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/http.c b/http.c
index 4c4a812..fee128b 100644
--- a/http.c
+++ b/http.c
@@ -735,9 +735,8 @@ static CURL *get_curl_handle(void)
allowed_protocols |= CURLPROTO_FTPS;
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS, allowed_protocols);
#else
- if (transport_restrict_protocols())
- warning("protocol restrictions not applied to curl redirects because\n"
- "your curl version is too old (>= 7.19.4)");
+ warning("protocol restrictions not applied to curl redirects because\n"
+ "your curl version is too old (>= 7.19.4)");
#endif
if (getenv("GIT_CURL_VERBOSE"))
curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);
diff --git a/transport.c b/transport.c
index 2c0ec76..186de9a 100644
--- a/transport.c
+++ b/transport.c
@@ -747,11 +747,6 @@ void transport_check_allowed(const char *type)
die("transport '%s' not allowed", type);
}
-int transport_restrict_protocols(void)
-{
- return !!protocol_whitelist();
-}
-
struct transport *transport_get(struct remote *remote, const char *url)
{
const char *helper;
diff --git a/transport.h b/transport.h
index b8e4ee8..f4998bc 100644
--- a/transport.h
+++ b/transport.h
@@ -164,12 +164,6 @@ int is_transport_allowed(const char *type);
*/
void transport_check_allowed(const char *type);
-/*
- * Returns true if the user has attempted to turn on protocol
- * restrictions at all.
- */
-int transport_restrict_protocols(void);
-
/* Transport options which apply to git:// and scp-style URLs */
/* The program to use on the remote side to send a pack */
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: bw/transport-protocol-policy
From: Jeff King @ 2016-12-01 19:46 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, git
In-Reply-To: <20161201193524.GC54082@google.com>
On Thu, Dec 01, 2016 at 11:35:24AM -0800, Brandon Williams wrote:
> > I wouldn't expect anyone to ever set GIT_PROTOCOL_FROM_USER=1, but it
> > does behave in a funny way here, overriding the "redirect" flag. I think
> > we'd want something more like:
> >
> > if (redirect < 0)
> > redirect = git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
> >
> > and then pass in "-1" from transport_check_allowed().
>
> I don't think I quite follow your solution but I came up with this:
>
> case PROTOCOL_ALLOW_USER_ONLY:
> return redirect ? 0 : git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
>
> Which should address the same issue.
I think mine was confused a bit by using the word "redirect". It was
really meant to be "from_user", and could take three values: definitely
yes, definitely no, and unknown (-1). And in the unknown case, we pull
the value from the environment.
Yours combines "definitely no" and "unknown" into a single value ("1" in
your case, but that is because "redirect" and "from_user" have inverted
logic from each other).
I think that is OK, as there isn't any case where a caller would want to
say "definitely no". The most they would say is "_I_ am not doing
anything to make you think this value is not from the user", but we
would still want to check the environment to see that nobody _else_ had
put in such a restriction.
-Peff
^ permalink raw reply
* [PATCH v6 4/4] transport: check if protocol can be used on a redirect
From: Brandon Williams @ 2016-12-01 19:48 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, peff, sbeller, bburky, jrnieder
In-Reply-To: <1480621447-52399-5-git-send-email-bmwill@google.com>
Add a the 'redirect' parameter to 'is_transport_allowed' which allows
callers to query if a transport protocol can be used on a redirect.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
http.c | 8 ++++----
transport.c | 6 +++---
transport.h | 7 ++++---
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/http.c b/http.c
index fee128b..d31ded8 100644
--- a/http.c
+++ b/http.c
@@ -725,13 +725,13 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_POST301, 1);
#endif
#if LIBCURL_VERSION_NUM >= 0x071304
- if (is_transport_allowed("http"))
+ if (is_transport_allowed("http", 1))
allowed_protocols |= CURLPROTO_HTTP;
- if (is_transport_allowed("https"))
+ if (is_transport_allowed("https", 1))
allowed_protocols |= CURLPROTO_HTTPS;
- if (is_transport_allowed("ftp"))
+ if (is_transport_allowed("ftp", 1))
allowed_protocols |= CURLPROTO_FTP;
- if (is_transport_allowed("ftps"))
+ if (is_transport_allowed("ftps", 1))
allowed_protocols |= CURLPROTO_FTPS;
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS, allowed_protocols);
#else
diff --git a/transport.c b/transport.c
index 186de9a..7c4a757 100644
--- a/transport.c
+++ b/transport.c
@@ -723,7 +723,7 @@ static enum protocol_allow_config get_protocol_config(const char *type)
return PROTOCOL_ALLOW_USER_ONLY;
}
-int is_transport_allowed(const char *type)
+int is_transport_allowed(const char *type, int redirect)
{
const struct string_list *whitelist = protocol_whitelist();
if (whitelist)
@@ -735,7 +735,7 @@ int is_transport_allowed(const char *type)
case PROTOCOL_ALLOW_NEVER:
return 0;
case PROTOCOL_ALLOW_USER_ONLY:
- return git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
+ return redirect ? 0 : git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
}
die("BUG: invalid protocol_allow_config type");
@@ -743,7 +743,7 @@ int is_transport_allowed(const char *type)
void transport_check_allowed(const char *type)
{
- if (!is_transport_allowed(type))
+ if (!is_transport_allowed(type, 0))
die("transport '%s' not allowed", type);
}
diff --git a/transport.h b/transport.h
index f4998bc..4bcf5d3 100644
--- a/transport.h
+++ b/transport.h
@@ -153,10 +153,11 @@ extern int transport_summary_width(const struct ref *refs);
struct transport *transport_get(struct remote *, const char *);
/*
- * Check whether a transport is allowed by the environment. Type should
- * generally be the URL scheme, as described in Documentation/git.txt
+ * Check whether a transport is allowed by the environment. Setting 'redirect'
+ * can be used to query if the transport can be used in a redirect. Type
+ * should generally be the URL scheme, as described in Documentation/git.txt
*/
-int is_transport_allowed(const char *type);
+int is_transport_allowed(const char *type, int redirect);
/*
* Check whether a transport is allowed by the environment,
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: [PATCH v6 4/4] transport: check if protocol can be used on a redirect
From: Brandon Williams @ 2016-12-01 19:49 UTC (permalink / raw)
To: git; +Cc: peff, sbeller, bburky, jrnieder
In-Reply-To: <1480621700-53222-1-git-send-email-bmwill@google.com>
On 12/01, Brandon Williams wrote:
> Add a the 'redirect' parameter to 'is_transport_allowed' which allows
> callers to query if a transport protocol can be used on a redirect.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
> http.c | 8 ++++----
> transport.c | 6 +++---
> transport.h | 7 ++++---
> 3 files changed, 11 insertions(+), 10 deletions(-)
>
Accidentally sent out an old version of just this patch. Here is the
updated one.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v6 4/4] transport: check if protocol can be used on a redirect
From: Jeff King @ 2016-12-01 19:50 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller, bburky, jrnieder
In-Reply-To: <1480621447-52399-5-git-send-email-bmwill@google.com>
On Thu, Dec 01, 2016 at 11:44:07AM -0800, Brandon Williams wrote:
> Add a the 'redirect' parameter to 'is_transport_allowed' which allows
> callers to query if a transport protocol can be used on a redirect.
s/a the/a/
> -int is_transport_allowed(const char *type)
> +int is_transport_allowed(const char *type, int redirect)
> {
> const struct string_list *whitelist = protocol_whitelist();
> if (whitelist)
> @@ -735,7 +735,7 @@ int is_transport_allowed(const char *type)
> case PROTOCOL_ALLOW_NEVER:
> return 0;
> case PROTOCOL_ALLOW_USER_ONLY:
> - return git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
> + return git_env_bool("GIT_PROTOCOL_FROM_USER", !redirect);
> }
This has the older logic still.
I'm not sure if we should call this "redirect" here. That's how it's
used by the curl code, but I think from the perspective of the transport
whitelist, it is really "are you overriding the from_user environment".
Calling it "from_user" may be confusing though, as the default value
would become "1", even though it means only "as far as I know this is
from the user, but maybe the environment says otherwise". So bizarrely,
I think calling it "not_from_user" is the clearest value.
-Peff
^ permalink raw reply
* Re: bw/transport-protocol-policy
From: Brandon Williams @ 2016-12-01 19:53 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20161201194621.dd3dbjv25ul6qgu5@sigill.intra.peff.net>
On 12/01, Jeff King wrote:
> On Thu, Dec 01, 2016 at 11:35:24AM -0800, Brandon Williams wrote:
>
> > > I wouldn't expect anyone to ever set GIT_PROTOCOL_FROM_USER=1, but it
> > > does behave in a funny way here, overriding the "redirect" flag. I think
> > > we'd want something more like:
> > >
> > > if (redirect < 0)
> > > redirect = git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
> > >
> > > and then pass in "-1" from transport_check_allowed().
> >
> > I don't think I quite follow your solution but I came up with this:
> >
> > case PROTOCOL_ALLOW_USER_ONLY:
> > return redirect ? 0 : git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
> >
> > Which should address the same issue.
>
> I think mine was confused a bit by using the word "redirect". It was
> really meant to be "from_user", and could take three values: definitely
> yes, definitely no, and unknown (-1). And in the unknown case, we pull
> the value from the environment.
>
> Yours combines "definitely no" and "unknown" into a single value ("1" in
> your case, but that is because "redirect" and "from_user" have inverted
> logic from each other).
>
> I think that is OK, as there isn't any case where a caller would want to
> say "definitely no". The most they would say is "_I_ am not doing
> anything to make you think this value is not from the user", but we
> would still want to check the environment to see that nobody _else_ had
> put in such a restriction.
>
> -Peff
Oh ok, I see what you were going for now. That may be a better
(clearer) solution then what I just sent out. Mostly because we can
maintain the same logic polarity and use the same vocabulary.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v6 4/4] transport: check if protocol can be used on a redirect
From: Junio C Hamano @ 2016-12-01 19:54 UTC (permalink / raw)
To: Jeff King; +Cc: Brandon Williams, git, sbeller, bburky, jrnieder
In-Reply-To: <20161201195031.fd4uwmvkyhk4so7i@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Dec 01, 2016 at 11:44:07AM -0800, Brandon Williams wrote:
>
>> Add a the 'redirect' parameter to 'is_transport_allowed' which allows
>> callers to query if a transport protocol can be used on a redirect.
>
> s/a the/a/
>
>> -int is_transport_allowed(const char *type)
>> +int is_transport_allowed(const char *type, int redirect)
>> {
>> const struct string_list *whitelist = protocol_whitelist();
>> if (whitelist)
>> @@ -735,7 +735,7 @@ int is_transport_allowed(const char *type)
>> case PROTOCOL_ALLOW_NEVER:
>> return 0;
>> case PROTOCOL_ALLOW_USER_ONLY:
>> - return git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
>> + return git_env_bool("GIT_PROTOCOL_FROM_USER", !redirect);
>> }
>
> This has the older logic still.
>
> I'm not sure if we should call this "redirect" here. That's how it's
> used by the curl code, but I think from the perspective of the transport
> whitelist, it is really "are you overriding the from_user environment".
>
> Calling it "from_user" may be confusing though, as the default value
> would become "1", even though it means only "as far as I know this is
> from the user, but maybe the environment says otherwise". So bizarrely,
> I think calling it "not_from_user" is the clearest value.
Bikeshedding: perhaps call it "unsafe" (in the sense that it is "not
known to be safe")?
^ permalink raw reply
* Re: [PATCH v6 4/4] transport: check if protocol can be used on a redirect
From: Jeff King @ 2016-12-01 19:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brandon Williams, git, sbeller, bburky, jrnieder
In-Reply-To: <xmqqbmwva0im.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 01, 2016 at 11:54:09AM -0800, Junio C Hamano wrote:
> > I'm not sure if we should call this "redirect" here. That's how it's
> > used by the curl code, but I think from the perspective of the transport
> > whitelist, it is really "are you overriding the from_user environment".
> >
> > Calling it "from_user" may be confusing though, as the default value
> > would become "1", even though it means only "as far as I know this is
> > from the user, but maybe the environment says otherwise". So bizarrely,
> > I think calling it "not_from_user" is the clearest value.
>
> Bikeshedding: perhaps call it "unsafe" (in the sense that it is "not
> known to be safe")?
That is definitely what we are going for, but it is vague about how it
is unsafe. :)
I think I may have converted Brandon in the other thread to my way of
thinking of it as a tristate[1]. That lets us call it "from_user", and
just do:
case PROTOCOL_ALLOW_FROM_USER:
if (from_user < 0)
from_user = git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
return from_user;
which is pretty clear. Nobody would ever pass "1" as from_user to the
function, but it does the sensible thing if they do.
-Peff
[1] The original I posted calling it "redirect" was totally bogus
because the logic between the two names is inverted.
^ permalink raw reply
* Re: [PATCH v6 4/4] transport: check if protocol can be used on a redirect
From: Brandon Williams @ 2016-12-01 20:01 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, sbeller, bburky, jrnieder
In-Reply-To: <20161201195902.td4zfolqpc3uwfgq@sigill.intra.peff.net>
On 12/01, Jeff King wrote:
> On Thu, Dec 01, 2016 at 11:54:09AM -0800, Junio C Hamano wrote:
>
> > > I'm not sure if we should call this "redirect" here. That's how it's
> > > used by the curl code, but I think from the perspective of the transport
> > > whitelist, it is really "are you overriding the from_user environment".
> > >
> > > Calling it "from_user" may be confusing though, as the default value
> > > would become "1", even though it means only "as far as I know this is
> > > from the user, but maybe the environment says otherwise". So bizarrely,
> > > I think calling it "not_from_user" is the clearest value.
> >
> > Bikeshedding: perhaps call it "unsafe" (in the sense that it is "not
> > known to be safe")?
>
> That is definitely what we are going for, but it is vague about how it
> is unsafe. :)
>
> I think I may have converted Brandon in the other thread to my way of
Yep, I've been converted :D
If we agree on that then I can make the change and resend the patch.
> thinking of it as a tristate[1]. That lets us call it "from_user", and
> just do:
>
> case PROTOCOL_ALLOW_FROM_USER:
> if (from_user < 0)
> from_user = git_env_bool("GIT_PROTOCOL_FROM_USER", 1);
> return from_user;
>
> which is pretty clear. Nobody would ever pass "1" as from_user to the
> function, but it does the sensible thing if they do.
>
> -Peff
>
> [1] The original I posted calling it "redirect" was totally bogus
> because the logic between the two names is inverted.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v2 1/1] convert: git cherry-pick -Xrenormalize did not work
From: Jacob Keller @ 2016-12-01 20:07 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Git mailing list, eevee.reply
In-Reply-To: <20161130170232.19685-1-tboegi@web.de>
On Wed, Nov 30, 2016 at 9:02 AM, <tboegi@web.de> wrote:
> From: Torsten Bögershausen <tboegi@web.de>
> diff --git a/convert.c b/convert.c
> index be91358..f8e4dfe 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -281,13 +281,13 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
> /*
> * If the file in the index has any CR in it, do not convert.
> * This is the new safer autocrlf handling.
> + - unless we want to renormalize in a merge or cherry-pick
Style nit, usually this line should begin with an aligned *? I think
it's not really that big a deal, though.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH 1/3] compat: add qsort_s()
From: Junio C Hamano @ 2016-12-01 20:14 UTC (permalink / raw)
To: Jeff King; +Cc: René Scharfe, Git List, Johannes Schindelin
In-Reply-To: <20161201193556.j2odwy3sepaxxq5a@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> To make matters more fun, apparently[1] there are multiple variants of
> qsort_r with different argument orders. _And_ apparently Microsoft
> defines qsort_s, but it's not quite the same thing. But all of that can
> be dealt with by having more specific flags (HAVE_GNU_QSORT_R, etc).
>
> It just seems like we should be able to do a better job of using the
> system qsort in many cases.
If we were to go that route, perhaps we shouldn't have HAVE_QSORT_S
so that Microsoft folks won't define it by mistake (instead perhaps
call it HAVE_ISO_QSORT_S or something).
I like your suggestion in general. The body of git_qsort_s() on
systems without ISO_QSORT_S can do
- GNU qsort_r() without any change in the parameters,
- Microsoft qsort_s() with parameter reordered, or
- Apple/BSD qsort_r() with parameter reordered.
and that would cover the major platforms.
Eh, wait. BSD and Microsoft have paramters reordered in the
callback comparison function. I suspect that would not fly very
well.
^ permalink raw reply
* Re: [PATCH 1/3] compat: add qsort_s()
From: Jeff King @ 2016-12-01 20:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: René Scharfe, Git List, Johannes Schindelin
In-Reply-To: <xmqq7f7j9zkd.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 01, 2016 at 12:14:42PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > To make matters more fun, apparently[1] there are multiple variants of
> > qsort_r with different argument orders. _And_ apparently Microsoft
> > defines qsort_s, but it's not quite the same thing. But all of that can
> > be dealt with by having more specific flags (HAVE_GNU_QSORT_R, etc).
> >
> > It just seems like we should be able to do a better job of using the
> > system qsort in many cases.
>
> If we were to go that route, perhaps we shouldn't have HAVE_QSORT_S
> so that Microsoft folks won't define it by mistake (instead perhaps
> call it HAVE_ISO_QSORT_S or something).
>
> I like your suggestion in general. The body of git_qsort_s() on
> systems without ISO_QSORT_S can do
>
> - GNU qsort_r() without any change in the parameters,
>
> - Microsoft qsort_s() with parameter reordered, or
>
> - Apple/BSD qsort_r() with parameter reordered.
>
> and that would cover the major platforms.
>
> Eh, wait. BSD and Microsoft have paramters reordered in the
> callback comparison function. I suspect that would not fly very
> well.
You can hack around it by passing a wrapper callback that flips the
arguments. Since we have a "void *" data pointer, that would point to a
struct holding the "real" callback and chaining to the original data
pointer.
It does incur the cost of an extra level of indirection for each
comparison, though (not just for each qsort call).
You could do it as zero-cost if you were willing to turn the comparison
function definition into a macro.
-Peff
^ permalink raw reply
* Re: [PATCH 1/3] compat: add qsort_s()
From: Junio C Hamano @ 2016-12-01 20:22 UTC (permalink / raw)
To: Jeff King; +Cc: René Scharfe, Git List, Johannes Schindelin
In-Reply-To: <xmqq7f7j9zkd.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Eh, wait. BSD and Microsoft have paramters reordered in the
> callback comparison function. I suspect that would not fly very
> well.
Hmm. We could do it like this, which may not be too bad.
#if APPLE_QSORT_R
struct apple_qsort_adapter {
int (*user_cmp)(const void *, const void *, void *);
void *user_ctx;
}
static int apple_qsort_adapter_cmp(void *ctx, const void *a, const void *b)
{
struct apple_qsort_adapter *wrapper_ctx = ctx;
return wrapper_ctx->user_cmp(a, b, wrapper_ctx->user_ctx);
}
#endif
int git_qsort_s(void *b, size_t n, size_t s,
int (*cmp)(const void *, const void *, void *), void *ctx)
{
if (!n)
return 0;
if (!b || !cmp)
return -1;
#if GNU_QSORT_R
qsort_r(b, n, s, cmp, ctx);
#elif APPLE_QSORT_R
{
struct appple_qsort_adapter a = { cmp, ctx };
qsort_r(b, n, s, &a, appple_qsort_adapter_cmp);
}
#endif
return 0;
}
^ permalink raw reply
* Re: [PATCH v2 1/1] convert: git cherry-pick -Xrenormalize did not work
From: Junio C Hamano @ 2016-12-01 20:24 UTC (permalink / raw)
To: Jacob Keller; +Cc: Torsten Bögershausen, Git mailing list, eevee.reply
In-Reply-To: <CA+P7+xoJb=SukbnJVJrXR6WV9+UtGnsn776KGkrHC7X-T_wZWg@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
> On Wed, Nov 30, 2016 at 9:02 AM, <tboegi@web.de> wrote:
>> From: Torsten Bögershausen <tboegi@web.de>
>> diff --git a/convert.c b/convert.c
>> index be91358..f8e4dfe 100644
>> --- a/convert.c
>> +++ b/convert.c
>> @@ -281,13 +281,13 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
>> /*
>> * If the file in the index has any CR in it, do not convert.
>> * This is the new safer autocrlf handling.
>> + - unless we want to renormalize in a merge or cherry-pick
>
> Style nit, usually this line should begin with an aligned *? I think
> it's not really that big a deal, though.
Yup, this is what I queued.
-- >8 --
From: Torsten Bögershausen <tboegi@web.de>
Date: Wed, 30 Nov 2016 18:02:32 +0100
Subject: [PATCH] convert: git cherry-pick -Xrenormalize did not work
Working with a repo that used to be all CRLF. At some point it
was changed to all LF, with `text=auto` in .gitattributes.
Trying to cherry-pick a commit from before the switchover fails:
$ git cherry-pick -Xrenormalize <commit>
fatal: CRLF would be replaced by LF in [path]
Commit 65237284 "unify the "auto" handling of CRLF" introduced
a regression:
Whenever crlf_action is CRLF_TEXT_XXX and not CRLF_AUTO_XXX,
SAFE_CRLF_RENORMALIZE was feed into check_safe_crlf(). This is
wrong because here everything else than SAFE_CRLF_WARN is treated as
SAFE_CRLF_FAIL.
Call check_safe_crlf() only if checksafe is SAFE_CRLF_WARN or
SAFE_CRLF_FAIL.
Reported-by: Eevee (Lexy Munroe) <eevee@veekun.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
convert.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/convert.c b/convert.c
index 077f5e601e..2f90f363c6 100644
--- a/convert.c
+++ b/convert.c
@@ -274,15 +274,16 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
if (convert_is_binary(len, &stats))
return 0;
/*
- * If the file in the index has any CR in it, do not convert.
- * This is the new safer autocrlf handling.
+ * If the file in the index has any CR in it, do not
+ * convert. This is the new safer autocrlf handling,
+ * unless we want to renormalize in a merge or
+ * cherry-pick.
*/
- if (checksafe == SAFE_CRLF_RENORMALIZE)
- checksafe = SAFE_CRLF_FALSE;
- else if (has_cr_in_index(path))
+ if ((checksafe != SAFE_CRLF_RENORMALIZE) && has_cr_in_index(path))
convert_crlf_into_lf = 0;
}
- if (checksafe && len) {
+ if ((checksafe == SAFE_CRLF_WARN ||
+ (checksafe == SAFE_CRLF_FAIL)) && len) {
struct text_stat new_stats;
memcpy(&new_stats, &stats, sizeof(new_stats));
/* simulate "git add" */
--
2.11.0-192-gbadfaabe38
^ permalink raw reply related
* Re: [PATCH 1/3] compat: add qsort_s()
From: Junio C Hamano @ 2016-12-01 20:25 UTC (permalink / raw)
To: Jeff King; +Cc: René Scharfe, Git List, Johannes Schindelin
In-Reply-To: <20161201201917.nqx3v5fl2ptl3bhr@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Dec 01, 2016 at 12:14:42PM -0800, Junio C Hamano wrote:
>
>> Eh, wait. BSD and Microsoft have paramters reordered in the
>> callback comparison function. I suspect that would not fly very
>> well.
>
> You can hack around it by passing a wrapper callback that flips the
> arguments.
Apparently our mails crossed.
^ permalink raw reply
* Re: [PATCH 1/3] compat: add qsort_s()
From: Jeff King @ 2016-12-01 20:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: René Scharfe, Git List, Johannes Schindelin
In-Reply-To: <xmqqy3zz8kmq.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 01, 2016 at 12:22:37PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Eh, wait. BSD and Microsoft have paramters reordered in the
> > callback comparison function. I suspect that would not fly very
> > well.
>
> Hmm. We could do it like this, which may not be too bad.
Heh. Exactly, but I was too lazy to write it out in my other email. :)
The no-cost version would be more like:
#ifdef APPLE_QSORT_R
#define DECLARE_CMP(func) int func(void *data, const void *va, const void *vb)
#else
#define DECLARE_CMP(func) int func(const void *va, const void *vb, void *data)
#endif
and then:
DECLARE_CMP(foocmp);
...
DECLARE_CMP(foocmp)
{
const struct foo *a = va, *b = vb;
... etc ...
}
-Peff
^ permalink raw reply
* [PATCHv3 1/5] submodule: use absolute path for computing relative path connecting
From: Stefan Beller @ 2016-12-01 20:25 UTC (permalink / raw)
To: pclouds; +Cc: git, bmwill, gitster, Stefan Beller
In-Reply-To: <20161201202554.19944-1-sbeller@google.com>
The current caller of connect_work_tree_and_git_dir passes
an absolute path for the `git_dir` parameter. In the future patch
we will also pass in relative path for `git_dir`. Extend the functionality
of connect_work_tree_and_git_dir to take relative paths for parameters.
We could work around this in the future patch by computing the absolute
path for the git_dir in the calling site, however accepting relative
paths for either parameter makes the API for this function much harder
to misuse.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
submodule.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/submodule.c b/submodule.c
index 6f7d883de9..66c5ce5a24 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1227,23 +1227,25 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
{
struct strbuf file_name = STRBUF_INIT;
struct strbuf rel_path = STRBUF_INIT;
- const char *real_work_tree = xstrdup(real_path(work_tree));
+ char *real_git_dir = xstrdup(real_path(git_dir));
+ char *real_work_tree = xstrdup(real_path(work_tree));
/* Update gitfile */
strbuf_addf(&file_name, "%s/.git", work_tree);
write_file(file_name.buf, "gitdir: %s",
- relative_path(git_dir, real_work_tree, &rel_path));
+ relative_path(real_git_dir, real_work_tree, &rel_path));
/* Update core.worktree setting */
strbuf_reset(&file_name);
- strbuf_addf(&file_name, "%s/config", git_dir);
+ strbuf_addf(&file_name, "%s/config", real_git_dir);
git_config_set_in_file(file_name.buf, "core.worktree",
- relative_path(real_work_tree, git_dir,
+ relative_path(real_work_tree, real_git_dir,
&rel_path));
strbuf_release(&file_name);
strbuf_release(&rel_path);
- free((void *)real_work_tree);
+ free(real_work_tree);
+ free(real_git_dir);
}
int parallel_submodules(void)
--
2.10.2.613.g22f2156
^ permalink raw reply related
* [PATCHv3 2/5] submodule helper: support super prefix
From: Stefan Beller @ 2016-12-01 20:25 UTC (permalink / raw)
To: pclouds; +Cc: git, bmwill, gitster, Stefan Beller
In-Reply-To: <20161201202554.19944-1-sbeller@google.com>
Just like main commands in Git, the submodule helper needs
access to the superproject prefix. Enable this in the git.c
but have its own fuse in the helper code by having a flag to
turn on the super prefix.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/submodule--helper.c | 31 ++++++++++++++++++++-----------
git.c | 2 +-
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5f9f..806e29ce4e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,21 +1076,24 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
return 0;
}
+#define SUPPORT_SUPER_PREFIX (1<<0)
+
struct cmd_struct {
const char *cmd;
int (*fn)(int, const char **, const char *);
+ int option;
};
static struct cmd_struct commands[] = {
- {"list", module_list},
- {"name", module_name},
- {"clone", module_clone},
- {"update-clone", update_clone},
- {"relative-path", resolve_relative_path},
- {"resolve-relative-url", resolve_relative_url},
- {"resolve-relative-url-test", resolve_relative_url_test},
- {"init", module_init},
- {"remote-branch", resolve_remote_submodule_branch}
+ {"list", module_list, 0},
+ {"name", module_name, 0},
+ {"clone", module_clone, 0},
+ {"update-clone", update_clone, 0},
+ {"relative-path", resolve_relative_path, 0},
+ {"resolve-relative-url", resolve_relative_url, 0},
+ {"resolve-relative-url-test", resolve_relative_url_test, 0},
+ {"init", module_init, 0},
+ {"remote-branch", resolve_remote_submodule_branch, 0}
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
@@ -1100,9 +1103,15 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
die(_("submodule--helper subcommand must be "
"called with a subcommand"));
- for (i = 0; i < ARRAY_SIZE(commands); i++)
- if (!strcmp(argv[1], commands[i].cmd))
+ for (i = 0; i < ARRAY_SIZE(commands); i++) {
+ if (!strcmp(argv[1], commands[i].cmd)) {
+ if (get_super_prefix() &&
+ !(commands[i].option & SUPPORT_SUPER_PREFIX))
+ die("%s doesn't support --super-prefix",
+ commands[i].cmd);
return commands[i].fn(argc - 1, argv + 1, prefix);
+ }
+ }
die(_("'%s' is not a valid submodule--helper "
"subcommand"), argv[1]);
diff --git a/git.c b/git.c
index efa1059fe0..98dcf6c518 100644
--- a/git.c
+++ b/git.c
@@ -493,7 +493,7 @@ static struct cmd_struct commands[] = {
{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
{ "stripspace", cmd_stripspace },
- { "submodule--helper", cmd_submodule__helper, RUN_SETUP },
+ { "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX},
{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
{ "tag", cmd_tag, RUN_SETUP },
{ "unpack-file", cmd_unpack_file, RUN_SETUP },
--
2.10.2.613.g22f2156
^ permalink raw reply related
* [PATCHv3 3/5] test-lib-functions.sh: teach test_commit -C <dir>
From: Stefan Beller @ 2016-12-01 20:25 UTC (permalink / raw)
To: pclouds; +Cc: git, bmwill, gitster, Stefan Beller
In-Reply-To: <20161201202554.19944-1-sbeller@google.com>
Specifically when setting up submodule tests, it comes in handy if
we can create commits in repositories that are not at the root of
the tested trash dir. Add "-C <dir>" similar to gits -C parameter
that will perform the operation in the given directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/test-lib-functions.sh | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fdaeb3a96b..579e812506 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -157,16 +157,21 @@ debug () {
GIT_TEST_GDB=1 "$@"
}
-# Call test_commit with the arguments "<message> [<file> [<contents> [<tag>]]]"
+# Call test_commit with the arguments
+# [-C <directory>] <message> [<file> [<contents> [<tag>]]]"
#
# This will commit a file with the given contents and the given commit
# message, and tag the resulting commit with the given tag name.
#
# <file>, <contents>, and <tag> all default to <message>.
+#
+# If the first argument is "-C", the second argument is used as a path for
+# the git invocations.
test_commit () {
notick= &&
signoff= &&
+ indir= &&
while test $# != 0
do
case "$1" in
@@ -176,21 +181,26 @@ test_commit () {
--signoff)
signoff="$1"
;;
+ -C)
+ indir="$2"
+ shift
+ ;;
*)
break
;;
esac
shift
done &&
+ indir=${indir:+"$indir"/} &&
file=${2:-"$1.t"} &&
- echo "${3-$1}" > "$file" &&
- git add "$file" &&
+ echo "${3-$1}" > "$indir$file" &&
+ git ${indir:+ -C "$indir"} add "$file" &&
if test -z "$notick"
then
test_tick
fi &&
- git commit $signoff -m "$1" &&
- git tag "${4:-$1}"
+ git ${indir:+ -C "$indir"} commit $signoff -m "$1" &&
+ git ${indir:+ -C "$indir"} tag "${4:-$1}"
}
# Call test_merge with the arguments "<message> <commit>", where <commit>
--
2.10.2.613.g22f2156
^ permalink raw reply related
* [PATCHv3 0/5] submodule embedgitdirs
From: Stefan Beller @ 2016-12-01 20:25 UTC (permalink / raw)
To: pclouds; +Cc: git, bmwill, gitster, Stefan Beller
v3:
* have a slightly more generic function "relocate_gitdir".
The recursion is strictly related to submodules, though.
* bail out if a submodule is using worktrees.
This also lays the groundwork for later doing the proper thing,
as worktree.h offers a function `get_submodule_worktrees(path)`
* nit by duy: use git_path instead of git_common_dir
* diff to v2 (as queued by Junio) below.
Stefan Beller (5):
submodule: use absolute path for computing relative path connecting
submodule helper: support super prefix
test-lib-functions.sh: teach test_commit -C <dir>
worktree: get worktrees from submodules
submodule: add embed-git-dir function
Documentation/git-submodule.txt | 14 ++++++
builtin/submodule--helper.c | 68 ++++++++++++++++++++++++-----
dir.c | 78 +++++++++++++++++++++++++++++++++
dir.h | 4 ++
git-submodule.sh | 7 ++-
git.c | 2 +-
submodule.c | 12 ++---
submodule.h | 1 -
t/t7412-submodule-embedgitdirs.sh | 92 +++++++++++++++++++++++++++++++++++++++
t/test-lib-functions.sh | 20 ++++++---
worktree.c | 40 ++++++++++++-----
worktree.h | 5 +++
12 files changed, 309 insertions(+), 34 deletions(-)
create mode 100755 t/t7412-submodule-embedgitdirs.sh
v2:
* fixed commit message for patch:
"submodule: use absolute path for computing relative path connecting"
* a new patch "submodule helper: support super prefix"
* redid the final patch with more tests and fixing bugs along the way
* "test-lib-functions.sh: teach test_commit -C <dir>" unchanged
v1:
The discussion of the submodule checkout series revealed to me that a command
is needed to move the git directory from the submodules working tree to be
embedded into the superprojects git directory.
So I wrote the code to intern the submodules git dir into the superproject,
but whilst writing the code I realized this could be valueable for our use
in testing too. So I exposed it via the submodule--helper. But as the
submodule helper ought to be just an internal API, we could also
offer it via the proper submodule command.
The command as it is has little value to the end user for now, but
breaking it out of the submodule checkout series hopefully makes review easier.
Thanks,
Stefan
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 75cdbf45b8..f2df166d15 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1081,11 +1081,14 @@ static int embed_git_dir(int argc, const char **argv, const char *prefix)
int i;
struct pathspec pathspec;
struct module_list list = MODULE_LIST_INIT;
+ unsigned flags = RELOCATE_GITDIR_RECURSE_SUBMODULES;
struct option embed_gitdir_options[] = {
OPT_STRING(0, "prefix", &prefix,
N_("path"),
N_("path into the working tree")),
+ OPT_BIT(0, "--recursive", &flags, N_("recurse into submodules"),
+ RELOCATE_GITDIR_RECURSE_SUBMODULES),
OPT_END()
};
@@ -1104,7 +1107,7 @@ static int embed_git_dir(int argc, const char **argv, const char *prefix)
return 1;
for (i = 0; i < list.nr; i++)
- migrate_submodule_gitdir(prefix, list.entries[i]->name, 1);
+ relocate_gitdir(prefix, list.entries[i]->name, flags);
return 0;
}
diff --git a/dir.c b/dir.c
index bfa8c8a9a5..7b3abc1340 100644
--- a/dir.c
+++ b/dir.c
@@ -15,6 +15,9 @@
#include "utf8.h"
#include "varint.h"
#include "ewah/ewok.h"
+#include "submodule-config.h"
+#include "run-command.h"
+#include "worktree.h"
struct path_simplify {
int len;
@@ -2748,3 +2751,78 @@ void untracked_cache_add_to_index(struct index_state *istate,
{
untracked_cache_invalidate_path(istate, path);
}
+
+/*
+ * Migrate the given submodule (and all its submodules recursively) from
+ * having its git directory within the working tree to the git dir nested
+ * in its superprojects git dir under modules/.
+ */
+void relocate_gitdir(const char *prefix, const char *path, unsigned flags)
+{
+ char *old_git_dir;
+ const char *new_git_dir;
+ const struct submodule *sub;
+ struct worktree **worktrees;
+ int i;
+
+ worktrees = get_submodule_worktrees(path);
+ for (i = 0; worktrees[i]; i++)
+ ;
+ if (i > 1)
+ die(_("relocate_gitdir for submodule with more than one worktree not supported"));
+
+ old_git_dir = xstrfmt("%s/.git", path);
+ if (read_gitfile(old_git_dir))
+ /* If it is an actual gitfile, it doesn't need migration. */
+ goto out;
+
+ sub = submodule_from_path(null_sha1, path);
+ if (!sub)
+ die(_("Could not lookup name for submodule '%s'"),
+ path);
+
+ new_git_dir = git_path("modules/%s", sub->name);
+ if (safe_create_leading_directories_const(new_git_dir) < 0)
+ die(_("could not create directory '%s'"), new_git_dir);
+
+ if (!prefix)
+ prefix = get_super_prefix();
+ printf("Migrating git directory of %s%s from\n'%s' to\n'%s'\n",
+ prefix ? prefix : "", path,
+ real_path(old_git_dir), new_git_dir);
+
+ if (rename(old_git_dir, new_git_dir) < 0)
+ die_errno(_("Could not migrate git directory from '%s' to '%s'"),
+ old_git_dir, new_git_dir);
+
+ connect_work_tree_and_git_dir(path, new_git_dir);
+
+out:
+ if (flags & RELOCATE_GITDIR_RECURSE_SUBMODULES) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (flags & ~RELOCATE_GITDIR_RECURSE_SUBMODULES)
+ die("BUG: we don't know how to pass the flags down?");
+
+ if (get_super_prefix())
+ strbuf_addstr(&sb, get_super_prefix());
+ strbuf_addstr(&sb, path);
+ strbuf_addch(&sb, '/');
+
+ cp.dir = path;
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ argv_array_pushl(&cp.args, "--super-prefix", sb.buf,
+ "submodule--helper",
+ "embed-git-dirs", NULL);
+ prepare_submodule_repo_env(&cp.env_array);
+ if (run_command(&cp))
+ die(_("Could not migrate git directory in submodule '%s'"),
+ path);
+
+ strbuf_release(&sb);
+ }
+
+ free(old_git_dir);
+}
diff --git a/dir.h b/dir.h
index 97c83bb383..0b5e99b21d 100644
--- a/dir.h
+++ b/dir.h
@@ -335,4 +335,8 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long
void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);
void add_untracked_cache(struct index_state *istate);
void remove_untracked_cache(struct index_state *istate);
+
+#define RELOCATE_GITDIR_RECURSE_SUBMODULES (1<<0)
+extern void relocate_gitdir(const char *prefix, const char *path, unsigned flags);
+
#endif
diff --git a/submodule.c b/submodule.c
index d330b567a3..66c5ce5a24 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1263,68 +1263,3 @@ void prepare_submodule_repo_env(struct argv_array *out)
}
argv_array_push(out, "GIT_DIR=.git");
}
-
-/*
- * Migrate the given submodule (and all its submodules recursively) from
- * having its git directory within the working tree to the git dir nested
- * in its superprojects git dir under modules/.
- */
-void migrate_submodule_gitdir(const char *prefix, const char *path,
- int recursive)
-{
- char *old_git_dir;
- const char *new_git_dir;
- const struct submodule *sub;
-
- old_git_dir = xstrfmt("%s/.git", path);
- if (read_gitfile(old_git_dir))
- /* If it is an actual gitfile, it doesn't need migration. */
- goto out;
-
- sub = submodule_from_path(null_sha1, path);
- if (!sub)
- die(_("Could not lookup name for submodule '%s'"),
- path);
-
- new_git_dir = git_common_path("modules/%s", sub->name);
- if (safe_create_leading_directories_const(new_git_dir) < 0)
- die(_("could not create directory '%s'"), new_git_dir);
-
- if (!prefix)
- prefix = get_super_prefix();
- printf("Migrating git directory of %s%s from\n'%s' to\n'%s'\n",
- prefix ? prefix : "", path,
- real_path(old_git_dir), new_git_dir);
-
- if (rename(old_git_dir, new_git_dir) < 0)
- die_errno(_("Could not migrate git directory from '%s' to '%s'"),
- old_git_dir, new_git_dir);
-
- connect_work_tree_and_git_dir(path, new_git_dir);
-
-out:
- if (recursive) {
- struct child_process cp = CHILD_PROCESS_INIT;
- struct strbuf sb = STRBUF_INIT;
-
- if (get_super_prefix())
- strbuf_addstr(&sb, get_super_prefix());
- strbuf_addstr(&sb, path);
- strbuf_addch(&sb, '/');
-
- cp.dir = path;
- cp.git_cmd = 1;
- cp.no_stdin = 1;
- argv_array_pushl(&cp.args, "--super-prefix", sb.buf,
- "submodule--helper",
- "embed-git-dirs", NULL);
- prepare_submodule_repo_env(&cp.env_array);
- if (run_command(&cp))
- die(_("Could not migrate git directory in submodule '%s'"),
- path);
-
- strbuf_release(&sb);
- }
-
- free(old_git_dir);
-}
diff --git a/submodule.h b/submodule.h
index e5975d1f3d..1e42222cff 100644
--- a/submodule.h
+++ b/submodule.h
@@ -74,7 +74,4 @@ int parallel_submodules(void);
* retaining any config in the environment.
*/
void prepare_submodule_repo_env(struct argv_array *out);
-
-extern void migrate_submodule_gitdir(const char *prefix,
- const char *path, int recursive);
#endif
diff --git a/t/t7412-submodule-embedgitdirs.sh b/t/t7412-submodule-embedgitdirs.sh
index 1cf5d4b4af..e3443b88cd 100755
--- a/t/t7412-submodule-embedgitdirs.sh
+++ b/t/t7412-submodule-embedgitdirs.sh
@@ -74,4 +74,19 @@ test_expect_success 'embedding the git dir fails for incomplete submodules' '
test_cmp expect.2 actual.2
'
+test_expect_success 'setup a submodule with multiple worktrees' '
+ # first create another unembedded git dir in a new submodule
+ git init sub3 &&
+ test_commit -C sub3 first &&
+ git submodule add ./sub3 &&
+ test_tick &&
+ git commit -m "add another submodule" &&
+ git -C sub3 worktree add ../sub3_second_work_tree
+'
+
+test_expect_success 'embed a submodule with multiple worktrees' '
+ test_must_fail git submodule embedgitdirs sub3 2>error &&
+ test_i18ngrep "not supported" error
+'
+
test_done
diff --git a/worktree.c b/worktree.c
index f7869f8d60..fb1f72bbf3 100644
--- a/worktree.c
+++ b/worktree.c
@@ -72,7 +72,7 @@ static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)
/**
* get the main worktree
*/
-static struct worktree *get_main_worktree(void)
+static struct worktree *get_main_worktree(const char *git_common_dir)
{
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
@@ -81,12 +81,12 @@ static struct worktree *get_main_worktree(void)
int is_bare = 0;
int is_detached = 0;
- strbuf_add_absolute_path(&worktree_path, get_git_common_dir());
+ strbuf_add_absolute_path(&worktree_path, git_common_dir);
is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
if (is_bare)
strbuf_strip_suffix(&worktree_path, "/.");
- strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
+ strbuf_addf(&path, "%s/HEAD", git_common_dir);
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
@@ -109,7 +109,8 @@ static struct worktree *get_main_worktree(void)
return worktree;
}
-static struct worktree *get_linked_worktree(const char *id)
+static struct worktree *get_linked_worktree(const char *git_common_dir,
+ const char *id)
{
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
@@ -120,7 +121,7 @@ static struct worktree *get_linked_worktree(const char *id)
if (!id)
die("Missing linked worktree name");
- strbuf_git_common_path(&path, "worktrees/%s/gitdir", id);
+ strbuf_addf(&path, "%s/worktrees/%s/gitdir", git_common_dir, id);
if (strbuf_read_file(&worktree_path, path.buf, 0) <= 0)
/* invalid gitdir file */
goto done;
@@ -133,7 +134,7 @@ static struct worktree *get_linked_worktree(const char *id)
}
strbuf_reset(&path);
- strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
+ strbuf_addf(&path, "%s/worktrees/%s/HEAD", git_common_dir, id);
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
@@ -173,7 +174,7 @@ static void mark_current_worktree(struct worktree **worktrees)
free(git_dir);
}
-struct worktree **get_worktrees(void)
+struct worktree **get_worktrees_internal(const char *git_common_dir)
{
struct worktree **list = NULL;
struct strbuf path = STRBUF_INIT;
@@ -183,10 +184,10 @@ struct worktree **get_worktrees(void)
list = xmalloc(alloc * sizeof(struct worktree *));
- if ((list[counter] = get_main_worktree()))
+ if ((list[counter] = get_main_worktree(git_common_dir)))
counter++;
- strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
+ strbuf_addf(&path, "%s/worktrees", git_common_dir);
dir = opendir(path.buf);
strbuf_release(&path);
if (dir) {
@@ -195,7 +196,7 @@ struct worktree **get_worktrees(void)
if (is_dot_or_dotdot(d->d_name))
continue;
- if ((linked = get_linked_worktree(d->d_name))) {
+ if ((linked = get_linked_worktree(git_common_dir, d->d_name))) {
ALLOC_GROW(list, counter + 1, alloc);
list[counter++] = linked;
}
@@ -209,6 +210,25 @@ struct worktree **get_worktrees(void)
return list;
}
+struct worktree **get_worktrees(void)
+{
+ return get_worktrees_internal(get_git_common_dir());
+}
+
+struct worktree **get_submodule_worktrees(const char *path)
+{
+ const char *submodule_common_dir;
+ struct strbuf sb = STRBUF_INIT;
+ struct worktree **ret;
+ strbuf_addf(&sb, "%s/.git", path);
+ submodule_common_dir = resolve_gitdir(sb.buf);
+
+ ret = get_worktrees_internal(submodule_common_dir);
+
+ strbuf_release(&sb);
+ return ret;
+}
+
const char *get_worktree_git_dir(const struct worktree *wt)
{
if (!wt)
diff --git a/worktree.h b/worktree.h
index 90e1311fa7..c93845516c 100644
--- a/worktree.h
+++ b/worktree.h
@@ -25,6 +25,11 @@ struct worktree {
*/
extern struct worktree **get_worktrees(void);
+/*
+ * Get the worktrees of a submodule named by `path`.
+ */
+extern struct worktree **get_submodule_worktrees(const char *path);
+
/*
* Return git dir of the worktree. Note that the path may be relative.
* If wt is NULL, git dir of current worktree is returned.
--
2.10.2.613.g22f2156
^ permalink raw reply related
* [PATCHv3 4/5] worktree: get worktrees from submodules
From: Stefan Beller @ 2016-12-01 20:25 UTC (permalink / raw)
To: pclouds; +Cc: git, bmwill, gitster, Stefan Beller
In-Reply-To: <20161201202554.19944-1-sbeller@google.com>
In a later patch we want to move around the the git directory of
a submodule. Both submodules as well as worktrees are involved in
placing git directories at unusual places, so their functionality
may collide. To react appropriately to situations where worktrees
in submodules are in use, offer a new function to query the
worktrees for submodules.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
worktree.c | 40 ++++++++++++++++++++++++++++++----------
worktree.h | 5 +++++
2 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/worktree.c b/worktree.c
index f7869f8d60..fb1f72bbf3 100644
--- a/worktree.c
+++ b/worktree.c
@@ -72,7 +72,7 @@ static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)
/**
* get the main worktree
*/
-static struct worktree *get_main_worktree(void)
+static struct worktree *get_main_worktree(const char *git_common_dir)
{
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
@@ -81,12 +81,12 @@ static struct worktree *get_main_worktree(void)
int is_bare = 0;
int is_detached = 0;
- strbuf_add_absolute_path(&worktree_path, get_git_common_dir());
+ strbuf_add_absolute_path(&worktree_path, git_common_dir);
is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
if (is_bare)
strbuf_strip_suffix(&worktree_path, "/.");
- strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
+ strbuf_addf(&path, "%s/HEAD", git_common_dir);
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
@@ -109,7 +109,8 @@ static struct worktree *get_main_worktree(void)
return worktree;
}
-static struct worktree *get_linked_worktree(const char *id)
+static struct worktree *get_linked_worktree(const char *git_common_dir,
+ const char *id)
{
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
@@ -120,7 +121,7 @@ static struct worktree *get_linked_worktree(const char *id)
if (!id)
die("Missing linked worktree name");
- strbuf_git_common_path(&path, "worktrees/%s/gitdir", id);
+ strbuf_addf(&path, "%s/worktrees/%s/gitdir", git_common_dir, id);
if (strbuf_read_file(&worktree_path, path.buf, 0) <= 0)
/* invalid gitdir file */
goto done;
@@ -133,7 +134,7 @@ static struct worktree *get_linked_worktree(const char *id)
}
strbuf_reset(&path);
- strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
+ strbuf_addf(&path, "%s/worktrees/%s/HEAD", git_common_dir, id);
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
@@ -173,7 +174,7 @@ static void mark_current_worktree(struct worktree **worktrees)
free(git_dir);
}
-struct worktree **get_worktrees(void)
+struct worktree **get_worktrees_internal(const char *git_common_dir)
{
struct worktree **list = NULL;
struct strbuf path = STRBUF_INIT;
@@ -183,10 +184,10 @@ struct worktree **get_worktrees(void)
list = xmalloc(alloc * sizeof(struct worktree *));
- if ((list[counter] = get_main_worktree()))
+ if ((list[counter] = get_main_worktree(git_common_dir)))
counter++;
- strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
+ strbuf_addf(&path, "%s/worktrees", git_common_dir);
dir = opendir(path.buf);
strbuf_release(&path);
if (dir) {
@@ -195,7 +196,7 @@ struct worktree **get_worktrees(void)
if (is_dot_or_dotdot(d->d_name))
continue;
- if ((linked = get_linked_worktree(d->d_name))) {
+ if ((linked = get_linked_worktree(git_common_dir, d->d_name))) {
ALLOC_GROW(list, counter + 1, alloc);
list[counter++] = linked;
}
@@ -209,6 +210,25 @@ struct worktree **get_worktrees(void)
return list;
}
+struct worktree **get_worktrees(void)
+{
+ return get_worktrees_internal(get_git_common_dir());
+}
+
+struct worktree **get_submodule_worktrees(const char *path)
+{
+ const char *submodule_common_dir;
+ struct strbuf sb = STRBUF_INIT;
+ struct worktree **ret;
+ strbuf_addf(&sb, "%s/.git", path);
+ submodule_common_dir = resolve_gitdir(sb.buf);
+
+ ret = get_worktrees_internal(submodule_common_dir);
+
+ strbuf_release(&sb);
+ return ret;
+}
+
const char *get_worktree_git_dir(const struct worktree *wt)
{
if (!wt)
diff --git a/worktree.h b/worktree.h
index 90e1311fa7..c93845516c 100644
--- a/worktree.h
+++ b/worktree.h
@@ -25,6 +25,11 @@ struct worktree {
*/
extern struct worktree **get_worktrees(void);
+/*
+ * Get the worktrees of a submodule named by `path`.
+ */
+extern struct worktree **get_submodule_worktrees(const char *path);
+
/*
* Return git dir of the worktree. Note that the path may be relative.
* If wt is NULL, git dir of current worktree is returned.
--
2.10.2.613.g22f2156
^ permalink raw reply related
* [PATCHv3 5/5] submodule: add embed-git-dir function
From: Stefan Beller @ 2016-12-01 20:25 UTC (permalink / raw)
To: pclouds; +Cc: git, bmwill, gitster, Stefan Beller
In-Reply-To: <20161201202554.19944-1-sbeller@google.com>
When a submodule has its git dir inside the working dir, the submodule
support for checkout that we plan to add in a later patch will fail.
Add functionality to migrate the git directory to be embedded
into the superprojects git directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-submodule.txt | 14 ++++++
builtin/submodule--helper.c | 39 ++++++++++++++++-
dir.c | 78 +++++++++++++++++++++++++++++++++
dir.h | 4 ++
git-submodule.sh | 7 ++-
submodule.h | 1 -
t/t7412-submodule-embedgitdirs.sh | 92 +++++++++++++++++++++++++++++++++++++++
7 files changed, 232 insertions(+), 3 deletions(-)
create mode 100755 t/t7412-submodule-embedgitdirs.sh
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index d841573475..34791cfc65 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -22,6 +22,7 @@ SYNOPSIS
[commit] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
+'git submodule' [--quiet] embedgitdirs [--] [<path>...]
DESCRIPTION
@@ -245,6 +246,19 @@ sync::
If `--recursive` is specified, this command will recurse into the
registered submodules, and sync any nested submodules within.
+embedgitdirs::
+ Move the git directory of submodules into its superprojects
+ `$GIT_DIR/modules` path and then connect the git directory and
+ its working directory by setting the `core.worktree` and adding
+ a .git file pointing to the git directory interned into the
+ superproject.
++
+A repository that was cloned independently and later added as a submodule or
+old setups have the submodules git directory inside the submodule instead of
+embedded into the superprojects git directory.
++
+This command is recursive by default.
+
OPTIONS
-------
-q::
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 806e29ce4e..f2df166d15 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,6 +1076,42 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
return 0;
}
+static int embed_git_dir(int argc, const char **argv, const char *prefix)
+{
+ int i;
+ struct pathspec pathspec;
+ struct module_list list = MODULE_LIST_INIT;
+ unsigned flags = RELOCATE_GITDIR_RECURSE_SUBMODULES;
+
+ struct option embed_gitdir_options[] = {
+ OPT_STRING(0, "prefix", &prefix,
+ N_("path"),
+ N_("path into the working tree")),
+ OPT_BIT(0, "--recursive", &flags, N_("recurse into submodules"),
+ RELOCATE_GITDIR_RECURSE_SUBMODULES),
+ OPT_END()
+ };
+
+ const char *const git_submodule_helper_usage[] = {
+ N_("git submodule--helper embed-git-dir [<path>...]"),
+ NULL
+ };
+
+ argc = parse_options(argc, argv, prefix, embed_gitdir_options,
+ git_submodule_helper_usage, 0);
+
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+
+ if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+ return 1;
+
+ for (i = 0; i < list.nr; i++)
+ relocate_gitdir(prefix, list.entries[i]->name, flags);
+
+ return 0;
+}
+
#define SUPPORT_SUPER_PREFIX (1<<0)
struct cmd_struct {
@@ -1093,7 +1129,8 @@ static struct cmd_struct commands[] = {
{"resolve-relative-url", resolve_relative_url, 0},
{"resolve-relative-url-test", resolve_relative_url_test, 0},
{"init", module_init, 0},
- {"remote-branch", resolve_remote_submodule_branch, 0}
+ {"remote-branch", resolve_remote_submodule_branch, 0},
+ {"embed-git-dirs", embed_git_dir, SUPPORT_SUPER_PREFIX}
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/dir.c b/dir.c
index bfa8c8a9a5..7b3abc1340 100644
--- a/dir.c
+++ b/dir.c
@@ -15,6 +15,9 @@
#include "utf8.h"
#include "varint.h"
#include "ewah/ewok.h"
+#include "submodule-config.h"
+#include "run-command.h"
+#include "worktree.h"
struct path_simplify {
int len;
@@ -2748,3 +2751,78 @@ void untracked_cache_add_to_index(struct index_state *istate,
{
untracked_cache_invalidate_path(istate, path);
}
+
+/*
+ * Migrate the given submodule (and all its submodules recursively) from
+ * having its git directory within the working tree to the git dir nested
+ * in its superprojects git dir under modules/.
+ */
+void relocate_gitdir(const char *prefix, const char *path, unsigned flags)
+{
+ char *old_git_dir;
+ const char *new_git_dir;
+ const struct submodule *sub;
+ struct worktree **worktrees;
+ int i;
+
+ worktrees = get_submodule_worktrees(path);
+ for (i = 0; worktrees[i]; i++)
+ ;
+ if (i > 1)
+ die(_("relocate_gitdir for submodule with more than one worktree not supported"));
+
+ old_git_dir = xstrfmt("%s/.git", path);
+ if (read_gitfile(old_git_dir))
+ /* If it is an actual gitfile, it doesn't need migration. */
+ goto out;
+
+ sub = submodule_from_path(null_sha1, path);
+ if (!sub)
+ die(_("Could not lookup name for submodule '%s'"),
+ path);
+
+ new_git_dir = git_path("modules/%s", sub->name);
+ if (safe_create_leading_directories_const(new_git_dir) < 0)
+ die(_("could not create directory '%s'"), new_git_dir);
+
+ if (!prefix)
+ prefix = get_super_prefix();
+ printf("Migrating git directory of %s%s from\n'%s' to\n'%s'\n",
+ prefix ? prefix : "", path,
+ real_path(old_git_dir), new_git_dir);
+
+ if (rename(old_git_dir, new_git_dir) < 0)
+ die_errno(_("Could not migrate git directory from '%s' to '%s'"),
+ old_git_dir, new_git_dir);
+
+ connect_work_tree_and_git_dir(path, new_git_dir);
+
+out:
+ if (flags & RELOCATE_GITDIR_RECURSE_SUBMODULES) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (flags & ~RELOCATE_GITDIR_RECURSE_SUBMODULES)
+ die("BUG: we don't know how to pass the flags down?");
+
+ if (get_super_prefix())
+ strbuf_addstr(&sb, get_super_prefix());
+ strbuf_addstr(&sb, path);
+ strbuf_addch(&sb, '/');
+
+ cp.dir = path;
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ argv_array_pushl(&cp.args, "--super-prefix", sb.buf,
+ "submodule--helper",
+ "embed-git-dirs", NULL);
+ prepare_submodule_repo_env(&cp.env_array);
+ if (run_command(&cp))
+ die(_("Could not migrate git directory in submodule '%s'"),
+ path);
+
+ strbuf_release(&sb);
+ }
+
+ free(old_git_dir);
+}
diff --git a/dir.h b/dir.h
index 97c83bb383..0b5e99b21d 100644
--- a/dir.h
+++ b/dir.h
@@ -335,4 +335,8 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long
void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);
void add_untracked_cache(struct index_state *istate);
void remove_untracked_cache(struct index_state *istate);
+
+#define RELOCATE_GITDIR_RECURSE_SUBMODULES (1<<0)
+extern void relocate_gitdir(const char *prefix, const char *path, unsigned flags);
+
#endif
diff --git a/git-submodule.sh b/git-submodule.sh
index a024a135d6..b7e124f340 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1131,6 +1131,11 @@ cmd_sync()
done
}
+cmd_embedgitdirs()
+{
+ git submodule--helper embed-git-dirs --prefix "$wt_prefix" "$@"
+}
+
# This loop parses the command line arguments to find the
# subcommand name to dispatch. Parsing of the subcommand specific
# options are primarily done by the subcommand implementations.
@@ -1140,7 +1145,7 @@ cmd_sync()
while test $# != 0 && test -z "$command"
do
case "$1" in
- add | foreach | init | deinit | update | status | summary | sync)
+ add | foreach | init | deinit | update | status | summary | sync | embedgitdirs)
command=$1
;;
-q|--quiet)
diff --git a/submodule.h b/submodule.h
index d9e197a948..1e42222cff 100644
--- a/submodule.h
+++ b/submodule.h
@@ -74,5 +74,4 @@ int parallel_submodules(void);
* retaining any config in the environment.
*/
void prepare_submodule_repo_env(struct argv_array *out);
-
#endif
diff --git a/t/t7412-submodule-embedgitdirs.sh b/t/t7412-submodule-embedgitdirs.sh
new file mode 100755
index 0000000000..e3443b88cd
--- /dev/null
+++ b/t/t7412-submodule-embedgitdirs.sh
@@ -0,0 +1,92 @@
+#!/bin/sh
+
+test_description='Test submodule embedgitdirs
+
+This test verifies that `git submodue embedgitdirs` moves a submodules git
+directory into the superproject.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup a real submodule' '
+ git init sub1 &&
+ test_commit -C sub1 first &&
+ git submodule add ./sub1 &&
+ test_tick &&
+ git commit -m superproject
+'
+
+test_expect_success 'embed the git dir' '
+ >expect.1 &&
+ >expect.2 &&
+ >actual.1 &&
+ >actual.2 &&
+ git status >expect.1 &&
+ git -C sub1 rev-parse HEAD >expect.2 &&
+ git submodule embedgitdirs &&
+ git fsck &&
+ test -f sub1/.git &&
+ test -d .git/modules/sub1 &&
+ git status >actual.1 &&
+ git -C sub1 rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
+test_expect_success 'setup nested submodule' '
+ git init sub1/nested &&
+ test_commit -C sub1/nested first_nested &&
+ git -C sub1 submodule add ./nested &&
+ test_tick &&
+ git -C sub1 commit -m "add nested" &&
+ git add sub1 &&
+ git commit -m "sub1 to include nested submodule"
+'
+
+test_expect_success 'embed the git dir in a nested submodule' '
+ git status >expect.1 &&
+ git -C sub1/nested rev-parse HEAD >expect.2 &&
+ git submodule embedgitdirs &&
+ test -f sub1/nested/.git &&
+ test -d .git/modules/sub1/modules/nested &&
+ git status >actual.1 &&
+ git -C sub1/nested rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
+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 'embedding the git dir fails for incomplete submodules' '
+ git status >expect.1 &&
+ git -C sub2 rev-parse HEAD >expect.2 &&
+ test_must_fail git submodule embedgitdirs &&
+ git -C sub2 fsck &&
+ test -d sub2/.git &&
+ git status >actual &&
+ git -C sub2 rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
+test_expect_success 'setup a submodule with multiple worktrees' '
+ # first create another unembedded git dir in a new submodule
+ git init sub3 &&
+ test_commit -C sub3 first &&
+ git submodule add ./sub3 &&
+ test_tick &&
+ git commit -m "add another submodule" &&
+ git -C sub3 worktree add ../sub3_second_work_tree
+'
+
+test_expect_success 'embed a submodule with multiple worktrees' '
+ test_must_fail git submodule embedgitdirs sub3 2>error &&
+ test_i18ngrep "not supported" error
+'
+
+test_done
--
2.10.2.613.g22f2156
^ permalink raw reply related
* [PATCH v7 0/4] transport protocol policy configuration
From: Brandon Williams @ 2016-12-01 20:25 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, peff, sbeller, bburky, jrnieder
In-Reply-To: <1480621447-52399-1-git-send-email-bmwill@google.com>
Changed the last patch in the series to use the parameter 'from_user' instead
of 'redirect'. This allows us to use the same logic polarity and maintain use
of the same vocabulary.
Brandon Williams (4):
lib-proto-disable: variable name fix
transport: add protocol policy config option
http: always warn if libcurl version is too old
transport: add from_user parameter to is_transport_allowed
Documentation/config.txt | 46 +++++++++++++
Documentation/git.txt | 38 ++++-------
git-submodule.sh | 12 ++--
http.c | 13 ++--
t/lib-proto-disable.sh | 142 ++++++++++++++++++++++++++++++++++++---
t/t5509-fetch-push-namespaces.sh | 1 +
t/t5802-connect-helper.sh | 1 +
transport.c | 84 ++++++++++++++++++++---
transport.h | 19 +++---
9 files changed, 289 insertions(+), 67 deletions(-)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply
* [PATCH v7 3/4] http: always warn if libcurl version is too old
From: Brandon Williams @ 2016-12-01 20:25 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, peff, sbeller, bburky, jrnieder
In-Reply-To: <1480623959-126129-1-git-send-email-bmwill@google.com>
Now that there are default "known-good" and "known-bad" protocols which
are allowed/disallowed by 'is_transport_allowed' we should always warn
the user that older versions of libcurl can't respect the allowed
protocols for redirects.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
http.c | 5 ++---
transport.c | 5 -----
transport.h | 6 ------
3 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/http.c b/http.c
index 4c4a812..fee128b 100644
--- a/http.c
+++ b/http.c
@@ -735,9 +735,8 @@ static CURL *get_curl_handle(void)
allowed_protocols |= CURLPROTO_FTPS;
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS, allowed_protocols);
#else
- if (transport_restrict_protocols())
- warning("protocol restrictions not applied to curl redirects because\n"
- "your curl version is too old (>= 7.19.4)");
+ warning("protocol restrictions not applied to curl redirects because\n"
+ "your curl version is too old (>= 7.19.4)");
#endif
if (getenv("GIT_CURL_VERBOSE"))
curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);
diff --git a/transport.c b/transport.c
index 2c0ec76..186de9a 100644
--- a/transport.c
+++ b/transport.c
@@ -747,11 +747,6 @@ void transport_check_allowed(const char *type)
die("transport '%s' not allowed", type);
}
-int transport_restrict_protocols(void)
-{
- return !!protocol_whitelist();
-}
-
struct transport *transport_get(struct remote *remote, const char *url)
{
const char *helper;
diff --git a/transport.h b/transport.h
index b8e4ee8..f4998bc 100644
--- a/transport.h
+++ b/transport.h
@@ -164,12 +164,6 @@ int is_transport_allowed(const char *type);
*/
void transport_check_allowed(const char *type);
-/*
- * Returns true if the user has attempted to turn on protocol
- * restrictions at all.
- */
-int transport_restrict_protocols(void);
-
/* Transport options which apply to git:// and scp-style URLs */
/* The program to use on the remote side to send a pack */
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox