* Re: [BUG] Git-GUI destroys the author name
From: bitjo @ 2016-12-12 20:59 UTC (permalink / raw)
To: git
In-Reply-To: <588ac268-0a1b-6787-e6a1-4164e7e9a784@b-i-t.de>
Addition to the bug
Proposal
https://github.com/git-for-windows/git/commit/30395c645adf21828bbccbdcd3c5c36c39f07050
should be considered and possibly implemented.
But as described in
https://github.com/git-for-windows/git/issues/761#issuecomment-221057828.
And then an implementation without regression is necessary.
^ permalink raw reply
* Re: [PATCH 2/2] tmp-objdir: quote paths we add to alternates
From: Johannes Sixt @ 2016-12-12 20:53 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Klaus Ethgen, git
In-Reply-To: <20161212195355.znqlu44lgnke3ltc@sigill.intra.peff.net>
Am 12.12.2016 um 20:53 schrieb Jeff King:
> Johannes, please let me know if I am wrong about skipping the test on
> !MINGW.
Thanks for being considerate. The !MINGW prerequisite is required for
the test as written.
> The appropriate check there would be ";" anyway, but I am not
> sure _that_ is allowed in paths, either.
That was also my line of thought. I tested earlier today whether a file
name can have ";", and the OS did not reject it bluntly. Let me see
whether I can adjust the test case for Windows.
-- Hannes
^ permalink raw reply
* Re: [PATCHv7 0/6] submodule absorbgitdirs
From: Stefan Beller @ 2016-12-12 20:37 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, git@vger.kernel.org, Duy Nguyen
In-Reply-To: <20161212203528.GA193413@google.com>
On Mon, Dec 12, 2016 at 12:35 PM, Brandon Williams <bmwill@google.com> wrote:
> On 12/12, Stefan Beller wrote:
>> v8:
>> * Change the worktree implementation: do not have an internal
>> submodule_get_worktrees, and count the number of worktrees, instead
>> directly look if there is a worktrees dir and check if there are any files
>> in there.
>> * reworded one test title slightly.
>> * interdiff to v7 (that is queued as origin/sb/submodule-embed-gitdir) below.
>>
>
> not important but your cover letter's subject is v7 instead of v8 :)
>
I realized that, after sending out.
When working on just one series on the mailing list,
I tend to reuse the coverletter and just adapt it, so I
would presume the diff stat is also broken.
^ permalink raw reply
* Re: [PATCHv7 0/6] submodule absorbgitdirs
From: Brandon Williams @ 2016-12-12 20:35 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, pclouds
In-Reply-To: <20161212190435.10358-1-sbeller@google.com>
On 12/12, Stefan Beller wrote:
> v8:
> * Change the worktree implementation: do not have an internal
> submodule_get_worktrees, and count the number of worktrees, instead
> directly look if there is a worktrees dir and check if there are any files
> in there.
> * reworded one test title slightly.
> * interdiff to v7 (that is queued as origin/sb/submodule-embed-gitdir) below.
>
not important but your cover letter's subject is v7 instead of v8 :)
--
Brandon Williams
^ permalink raw reply
* Re: git add -p with new file
From: Stephan Beyer @ 2016-12-12 20:31 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: Ariel, git
In-Reply-To: <20161211130034.ygj5l2gbx33uknlk@sigill.intra.peff.net>
Hi,
On 12/11/2016 02:00 PM, Jeff King wrote:
> On Sat, Dec 10, 2016 at 02:04:33PM -0800, Junio C Hamano wrote:
>> Jeff King <peff@peff.net> writes:
>>> On Fri, Dec 09, 2016 at 01:43:24PM -0500, Ariel wrote:
>>> ...
>>>> But it doesn't have to be that way. You could make add -p identical to add
>>>> without options, except the -p prompts to review diffs first.
>>>
>>> The question is whether you would annoy people using "-p" if you started
>>> including untracked files by default. I agree because it's inherently an
>>> interactive process that we can be looser with backwards compatibility.
>>
>> It might be interactive, but it will be irritating that we suddenly
>> have to see hundreds of lines in an untracked file before we are
>> asked to say "no I do not want to add this file" and have to do so
>> for all the untracked files that happen to match the given pathspec.
>>
>> It might make it less irritating if one of the interactive choices
>> offered in the first prompt were N that tells the command: "No,
>> ignore all the untracked paths", though. I dunno.
>
> Yeah, I agree dumping the contents automatically is annoying. Ariel
> suggested asking twice about each path, which sounds clunky to me. I'd
> probably give a simple question, with an option to dump the contents.
> Like:
>
> $ echo foo >untracked
> $ git add -p
> New file: untracked
> Stage this file [y,n,v,q,a,d,/,e,?]? v <-- user types 'v' for "view"
I am also a "git add -p"-only user (except for new files and merges).
However, I usually keep a lot of untracked files in my repositories.
Files that I do not (git)ignore because I want to see them when I type
"git status".
Hence, the imagination only that "git add -p" starts to ask me for each
untracked file feels like a lot of annoying "n" presses. I could imagine
that it is okay-ish when it asks about the untracked files *after* all
tracked paths have been processed (I guess this has been proposed
before), so that I can safely quit.
I am also not really sure what problem this feature is trying to solve.
If the "problem"(?) is that it should act more like "git add" instead of
"git add -u", for whatever reason, this may be fine (but the
configuration option is a must-have then).
For me, I often had the problem that I simply forgot to add new
files...¹ Still I doubt that one would benefit from such a feature
because either:
- you do not have many untracked files (unlike me). You will see the
untracked file (that should be tracked) in the "git status" output when
you edit the commit message², then you abort-add-commit or
commit-add-amend and everything is fine.
Or:
- you have a lot of untracked files (like me). You won't see the
untracked file (that should be tracked) in the "git status" output (you
ignore it because the list is so long) and you won't see it in the "git
add -p" run because you quit before seeing that file.
So I have mixed feelings...
> I'd also probably add interactive.showUntracked to make the whole thing
> optional (but I think it would be OK to default it to on).
Hm, "interactive.showUntracked" is a confusing name because "git add -i"
(interactive) already handles untracked files.
Best
Stephan
¹ I do not have that problem any more because I got used to add new
files right after saving, in a very early state, and then I simply use
"git add -p"...
² Unless you use git commit -m ... But using "git commit -m" without
"git status" before is user-issue anyway.
^ permalink raw reply
* Re: git add -p with new file
From: Stephan Beyer @ 2016-12-12 20:24 UTC (permalink / raw)
To: Ariel, Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <alpine.DEB.2.11.1612091322290.13185@cherryberry.dsgml.com>
Hi Ariel,
On 12/09/2016 07:26 PM, Ariel wrote:
> On Wed, 7 Dec 2016, Duy Nguyen wrote:
>> We could improve it a bit, suggesting the user to do git add -N. But
>> is there a point of using -p on a new file?
>
> I got into the habit of always using -p as a way of checking my diffs
> before committing,
Not commenting on the main issue here, but if you want to check your new
files with a "git add -p"-like tool, you can do
git add <untracked files>
git reset -p
(this is unstaging, but it's still useful)
Best
Stephan
^ permalink raw reply
* [BUG] Git-GUI destroys the author name
From: bitjo @ 2016-12-12 20:05 UTC (permalink / raw)
To: git
Since Git version 2.11, git gui destroys the author name when 'git
commit amend' is executed and the author name contains UTF-8-specific
characters.
This is a known issue from 'Git for Windows' and now a
platform-independent bug.
Please refer
- https://github.com/git-for-windows/git/issues/761
- https://github.com/git-for-windows/git/issues/761#issuecomment-220792200
- https://github.com/git-for-windows/git/issues/761#issuecomment-266519859
From my point of view, this is a severe error since the repostory is
changed incorrectly. Users of git gui will usually not be able to
correct this error with git commands.
^ permalink raw reply
* Re: [PATCH 1/3] compat: add qsort_s()
From: Jeff King @ 2016-12-12 19:57 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Git List, Johannes Schindelin
In-Reply-To: <b8aa28b1-e645-4cea-cc91-96f62fee6118@web.de>
On Mon, Dec 12, 2016 at 08:51:14PM +0100, René Scharfe wrote:
> It's kinda cool to have a bespoke compatibility layer for major platforms,
> but the more I think about it the less I can answer why we would want that.
> Safety, reliability and performance can't be good reasons -- if our fallback
> function lacks in these regards then we have to improve it in any case.
There may be cases that we don't want to support because of portability
issues. E.g., if your libc has an assembly-optimized qsort() we wouldn't
want to replicate that.
I dunno. I am not that opposed to just saying "forget libc qsort, we
always use our internal one which is consistent, performant, and safe".
But when I suggested something similar for our regex library, I seem to
recall there were complaints.
-Peff
^ permalink raw reply
* [PATCH 2/2] tmp-objdir: quote paths we add to alternates
From: Jeff King @ 2016-12-12 19:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <20161212194929.bdcihf7orjabzb2h@sigill.intra.peff.net>
Commit 722ff7f87 (receive-pack: quarantine objects until
pre-receive accepts, 2016-10-03) regressed pushes to
repositories with colon (or semi-colon in Windows in them)
because it adds the repository's main object directory to
GIT_ALTERNATE_OBJECT_DIRECTORIES. The receiver interprets
the colon as a delimiter, not as part of the path, and
index-pack is unable to find objects which it needs to
resolve deltas.
The previous commit introduced a quoting mechanism for the
alternates list; let's use it here to cover this case. We'll
avoid quoting when we can, though. This alternate setup is
also used when calling hooks, so it's possible that the user
may call older git implementations which don't understand
the quoting mechanism. By quoting only when necessary, this
setup will continue to work unless the user _also_ has a
repository whose path contains the delimiter.
Signed-off-by: Jeff King <peff@peff.net>
---
Johannes, please let me know if I am wrong about skipping the test on
!MINGW. The appropriate check there would be ";" anyway, but I am not
sure _that_ is allowed in paths, either.
t/t5547-push-quarantine.sh | 19 +++++++++++++++++++
tmp-objdir.c | 18 +++++++++++++++++-
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
index 1e5d32d06..6275ec807 100755
--- a/t/t5547-push-quarantine.sh
+++ b/t/t5547-push-quarantine.sh
@@ -33,4 +33,23 @@ test_expect_success 'rejected objects are removed' '
test_cmp expect actual
'
+# MINGW does not allow colons in pathnames in the first place
+test_expect_success !MINGW 'push to repo path with colon' '
+ # The interesting failure case here is when the
+ # receiving end cannot access its original object directory,
+ # so make it likely for us to generate a delta by having
+ # a non-trivial file with multiple versions.
+
+ test-genrandom foo 4096 >file.bin &&
+ git add file.bin &&
+ git commit -m bin &&
+ git clone --bare . xxx:yyy.git &&
+
+ echo change >>file.bin &&
+ git commit -am change &&
+ # Note that we have to use the full path here, or it gets confused
+ # with the ssh host:path syntax.
+ git push "$PWD/xxx:yyy.git" HEAD
+'
+
test_done
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 64435f23a..b2d9280f1 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -5,6 +5,7 @@
#include "string-list.h"
#include "strbuf.h"
#include "argv-array.h"
+#include "quote.h"
struct tmp_objdir {
struct strbuf path;
@@ -79,12 +80,27 @@ static void remove_tmp_objdir_on_signal(int signo)
*/
static void env_append(struct argv_array *env, const char *key, const char *val)
{
- const char *old = getenv(key);
+ struct strbuf quoted = STRBUF_INIT;
+ const char *old;
+ /*
+ * Avoid quoting if it's not necessary, for maximum compatibility
+ * with older parsers which don't understand the quoting.
+ */
+ if (*val == '"' || strchr(val, PATH_SEP)) {
+ strbuf_addch("ed, '"');
+ quote_c_style(val, "ed, NULL, 1);
+ strbuf_addch("ed, '"');
+ val = quoted.buf;
+ }
+
+ old = getenv(key);
if (!old)
argv_array_pushf(env, "%s=%s", key, val);
else
argv_array_pushf(env, "%s=%s%c%s", key, old, PATH_SEP, val);
+
+ strbuf_release("ed);
}
static void env_replace(struct argv_array *env, const char *key, const char *val)
--
2.11.0.203.g6657065
^ permalink raw reply related
* [PATCH 1/2] alternates: accept double-quoted paths
From: Jeff King @ 2016-12-12 19:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <20161212194929.bdcihf7orjabzb2h@sigill.intra.peff.net>
We read lists of alternates from objects/info/alternates
files (delimited by newline), as well as from the
GIT_ALTERNATE_OBJECT_DIRECTORIES environment variable
(delimited by colon or semi-colon, depending on the
platform).
There's no mechanism for quoting the delimiters, so it's
impossible to specify an alternate path that contains a
colon in the environment, or one that contains a newline in
a file. We've lived with that restriction for ages because
both alternates and filenames with colons are relatively
rare, and it's only a problem when the two meet. But since
722ff7f87 (receive-pack: quarantine objects until
pre-receive accepts, 2016-10-03), which builds on the
alternates system, every push causes the receiver to set
GIT_ALTERNATE_OBJECT_DIRECTORIES internally.
It would be convenient to have some way to quote the
delimiter so that we can represent arbitrary paths.
The simplest thing would be an escape character before a
quoted delimiter (e.g., "\:" as a literal colon). But that
creates a backwards compatibility problem: any path which
uses that escape character is now broken, and we've just
shifted the problem. We could choose an unlikely escape
character (e.g., something from the non-printable ASCII
range), but that's awkward to use.
Instead, let's treat names as unquoted unless they begin
with a double-quote, in which case they are interpreted via
our usual C-stylke quoting rules. This also breaks
backwards-compatibility, but in a smaller way: it only
matters if your file has a double-quote as the very _first_
character in the path (whereas an escape character is a
problem anywhere in the path). It's also consistent with
many other parts of git, which accept either a bare pathname
or a double-quoted one, and the sender can choose to quote
or not as required.
Signed-off-by: Jeff King <peff@peff.net>
---
This also lets you specify paths that start with "#", or ones that
contain newlines in an alternates file, though I doubt anybody really
cares much in practice. I didn't even bother to add them to the test
suite, mostly because of the compatibility hassle (though I guess we
could just mark them with !MINGW and be OK).
Documentation/git.txt | 6 ++++++
sha1_file.c | 47 ++++++++++++++++++++++++++++++++++++-----------
t/t5615-alternate-env.sh | 18 ++++++++++++++++++
3 files changed, 60 insertions(+), 11 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index af191c51b..98033302b 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -871,6 +871,12 @@ Git so take care if using a foreign front-end.
specifies a ":" separated (on Windows ";" separated) list
of Git object directories which can be used to search for Git
objects. New objects will not be written to these directories.
++
+ Entries that begin with `"` (double-quote) will be interpreted
+ as C-style quoted paths, removing leading and trailing
+ double-quotes and respecting backslash escapes. E.g., the value
+ `"path-with-\"-and-:-in-it":vanilla-path` has two paths:
+ `path-with-"-and-:-in-it` and `vanilla-path`.
`GIT_DIR`::
If the `GIT_DIR` environment variable is set then it
diff --git a/sha1_file.c b/sha1_file.c
index 9c86d1924..117307185 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -26,6 +26,7 @@
#include "mru.h"
#include "list.h"
#include "mergesort.h"
+#include "quote.h"
#ifndef O_NOATIME
#if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
@@ -329,13 +330,40 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
return 0;
}
+static const char *parse_alt_odb_entry(const char *string,
+ int sep,
+ struct strbuf *out)
+{
+ const char *end;
+
+ strbuf_reset(out);
+
+ if (*string == '#') {
+ /* comment; consume up to next separator */
+ end = strchrnul(string, sep);
+ } else if (*string == '"' && !unquote_c_style(out, string, &end)) {
+ /*
+ * quoted path; unquote_c_style has copied the
+ * data for us and set "end". Broken quoting (e.g.,
+ * an entry that doesn't end with a quote) falls
+ * back to the unquoted case below.
+ */
+ } else {
+ /* normal, unquoted path */
+ end = strchrnul(string, sep);
+ strbuf_add(out, string, end - string);
+ }
+
+ if (*end)
+ end++;
+ return end;
+}
+
static void link_alt_odb_entries(const char *alt, int len, int sep,
const char *relative_base, int depth)
{
- struct string_list entries = STRING_LIST_INIT_NODUP;
- char *alt_copy;
- int i;
struct strbuf objdirbuf = STRBUF_INIT;
+ struct strbuf entry = STRBUF_INIT;
if (depth > 5) {
error("%s: ignoring alternate object stores, nesting too deep.",
@@ -348,16 +376,13 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,
die("unable to normalize object directory: %s",
objdirbuf.buf);
- alt_copy = xmemdupz(alt, len);
- string_list_split_in_place(&entries, alt_copy, sep, -1);
- for (i = 0; i < entries.nr; i++) {
- const char *entry = entries.items[i].string;
- if (entry[0] == '\0' || entry[0] == '#')
+ while (*alt) {
+ alt = parse_alt_odb_entry(alt, sep, &entry);
+ if (!entry.len)
continue;
- link_alt_odb_entry(entry, relative_base, depth, objdirbuf.buf);
+ link_alt_odb_entry(entry.buf, relative_base, depth, objdirbuf.buf);
}
- string_list_clear(&entries, 0);
- free(alt_copy);
+ strbuf_release(&entry);
strbuf_release(&objdirbuf);
}
diff --git a/t/t5615-alternate-env.sh b/t/t5615-alternate-env.sh
index eec4137ca..69fd8f891 100755
--- a/t/t5615-alternate-env.sh
+++ b/t/t5615-alternate-env.sh
@@ -68,4 +68,22 @@ test_expect_success 'access alternate via relative path (subdir)' '
EOF
'
+# set variables outside test to avoid quote insanity; the \057 is '/',
+# which doesn't need quoting, but just confirms that de-quoting
+# is working.
+quoted='"one.git\057objects"'
+unquoted='two.git/objects'
+test_expect_success 'mix of quoted and unquoted alternates' '
+ check_obj "$quoted:$unquoted" <<-EOF
+ $one blob
+ $two blob
+'
+
+test_expect_success 'broken quoting falls back to interpreting raw' '
+ mv one.git \"one.git &&
+ check_obj \"one.git/objects <<-EOF
+ $one blob
+ EOF
+'
+
test_done
--
2.11.0.203.g6657065
^ permalink raw reply related
* Re: [PATCH 1/3] compat: add qsort_s()
From: René Scharfe @ 2016-12-12 19:51 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: Git List, Johannes Schindelin
In-Reply-To: <xmqqy3zz8kmq.fsf@gitster.mtv.corp.google.com>
Am 01.12.2016 um 21:22 schrieb Junio C Hamano:
> 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.
It's kinda cool to have a bespoke compatibility layer for major
platforms, but the more I think about it the less I can answer why we
would want that. Safety, reliability and performance can't be good
reasons -- if our fallback function lacks in these regards then we have
to improve it in any case.
Text size could be a valid reason, but the full function only adds a bit
more than 2KB to the unstripped git binary.
The flip side is we'd build an ifdef maze that's harder to read and a
lot more difficult to test.
What do we get in return for that additional complexity?
> #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
Nit: The fallback for non-GNU, non-Apple systems is missing here, but
the idea is illustrated clearly enough.
> return 0;
> }
>
^ permalink raw reply
* [PATCH 0/2] handling alternates paths with colons
From: Jeff King @ 2016-12-12 19:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <20161210085133.2pnkz6eqlxoxdckg@sigill.intra.peff.net>
On Sat, Dec 10, 2016 at 03:51:33AM -0500, Jeff King wrote:
> So here's the array of options, as I see it:
>
> 1. Disable quarantine by default; only turn it on if you don't have
> any of the funny corner cases.
>
> 2. Introduce an extra single-item environment variable that gets
> appended to the list of alternates, and side-steps quoting issues.
>
> 3. Introduce a new variable that can hold an alternate list, and
> either teach it reasonable quoting semantics, or give it a
> less-common separator.
>
> 4. Introduce quoting to the existing variable, but make the quoting
> character sufficiently obscure that nobody cares about the lack of
> backwards compatibility.
So I started on (4), but writing the user-facing documentation made me
throw up in my mouth a little. Fortunately that inspired me to come up
with a fifth option: introduce a quoting mechanism that needs a more
obscure signature to kick in, but once kicked-in, uses a less obscure
syntax.
So here are patches that do that. It kicks in only when the first
character of a path is a double-quote, and then expects the usual
C-style quoting. My reasoning is that we wouldn't want to sacrifice
'"' for ':', but it's probably OK if we only care about '"' at the very
beginning of a path. And it's consistent with many other parts of git
which allow optional quoting.
Thoughts?
[1/2]: alternates: accept double-quoted paths
[2/2]: tmp-objdir: quote paths we add to alternates
Documentation/git.txt | 6 ++++++
sha1_file.c | 47 +++++++++++++++++++++++++++++++++++-----------
t/t5547-push-quarantine.sh | 19 +++++++++++++++++++
t/t5615-alternate-env.sh | 18 ++++++++++++++++++
tmp-objdir.c | 18 +++++++++++++++++-
5 files changed, 96 insertions(+), 12 deletions(-)
-Peff
^ permalink raw reply
* Re: [RFC PATCH] send-email: allow a custom hook to prevent sending email
From: Stefan Beller @ 2016-12-12 19:35 UTC (permalink / raw)
To: Jeff King; +Cc: Brandon Williams, git@vger.kernel.org
In-Reply-To: <20161210091333.rhssskrvzjqnzelb@sigill.intra.peff.net>
On Sat, Dec 10, 2016 at 1:13 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Dec 09, 2016 at 12:34:49PM -0800, Stefan Beller wrote:
>
>> My first perl contribution to Git. :)
>
> Yes, I have some style suggestions below. :)
>
>> Marked as RFC to gauge general interest before writing tests and
>> documentation.
>
> It's hard to evaluate without seeing an example of what you'd actually
> want to put into a hook.
>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index da81be40cb..d3ebf666c3 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -815,6 +815,15 @@ if (!$force) {
>> . "Pass --force if you really want to send.\n";
>> }
>> }
>> + my @hook = ( $ENV{GIT_DIR}.'hooks/send-email', $f )
>
> It's awkward to use a list here, when you just end up accessing
> $hook[0]. You can form the list on the fly when you call system(). You
> also probably are missing a trailing "/".
>
> I'm not sure that $GIT_DIR is consistently set; you probably want to
> look at "git rev-parse --git-dir" here. But I think we make a Git
> repository object earlier, and you can just do:
>
> my $hook = join('/', $repo->repo_path(), 'hooks/send-email');
>
> Or you can just do string concatenation:
>
> my $hook = $repo->repo_path() . '/hooks/send-email';
>
> If I were writing repo_path myself, I'd probably allow:
>
> my $hook = $repo->repo_path('hooks/send-email');
>
> like we do with git_path() in the C code. It wouldn't be hard to add.
>
>> + if( -x $hook[0] ) {
>
> Funny whitespace. I think:
>
> if (-x $hook)
>
> would be more usual for us.
>
>> + unless( system( @hook ) == 0 )
>> + {
>> + die "Refusing to send because the patch\n\t$f\n"
>> + . "was refused by the send-email hook."
>> + . "Pass --force if you really want to send.\n";
>> + }
>
> I like "unless" as a trailing one-liner conditional for edge cases,
> like:
>
> do_this($foo) unless $foo < 5;
>
> but I think here it just makes things more Perl-ish than our code base
> usually goes for. Probably:
>
> if (system($hook, $f) != 0) {
> die ...
> }
>
> would be more usual for us (in a more Perl-ish code base I'd probably
> write "system(...) and die ...").
>
> -Peff
Oh, that is quite some feedback on how not to write perl.
Thanks for pointing out how you would do it. My patch was heavily inspired
by git-cvsserver.perl:1720 so maybe that is not the best example to follow.
While trying to figure out the right place, where to put the actual code, I was
wondering about the possible interaction with it, e.g. looking at
output like this
$ git send-email 00* --cc=list --cc=bmwill --cc=duy --to=jch
0000-cover-letter.patch
0001-submodule-use-absolute-path-for-computing-relative-p.patch
0002-submodule-helper-support-super-prefix.patch
0003-test-lib-functions.sh-teach-test_commit-C-dir.patch
0004-worktree-check-if-a-submodule-uses-worktrees.patch
0005-move-connect_work_tree_and_git_dir-to-dir.h.patch
0006-submodule-add-absorb-git-dir-function.patch
(mbox) Adding cc: Stefan Beller <sbeller@google.com> from line 'From:
Stefan Beller <sbeller@google.com>'
From: Stefan Beller <sbeller@google.com>
To: gitster@pobox.com
Cc: git@vger.kernel.org,
bmwill@google.com,
pclouds@gmail.com,
Stefan Beller <sbeller@google.com>
Subject: [PATCHv7 0/6] submodule absorbgitdirs
Date: Mon, 12 Dec 2016 11:04:29 -0800
Message-Id: <20161212190435.10358-1-sbeller@google.com>
X-Mailer: git-send-email 2.11.0.rc2.49.ge1f3b0c.dirty
Send this email? ([y]es|[n]o|[q]uit|[a]ll):
---
This is usually where I just say "a" and carry on with life.
The hook would ideally reformat the last lines to be
---
X-Mailer: git-send-email 2.11.0.rc2.49.ge1f3b0c.dirty
send-email hook thinks it is a bad idea to send out this series:
<output of hook, e.g.
referencing a typo in patch 5.
or a mismatch of patch version numbers>
Send this email? ([y]es|[n]o|[q]uit|[a]ll):
---
So I still need to look around to see the big picture for this
patch to be implemented ideal.
^ permalink raw reply
* Re: [PATCH 1/3] update_unicode.sh: update the uniset repo if it exists
From: Beat Bolli @ 2016-12-12 19:24 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <ca10a51a-0fab-e4a4-8d7d-035673af4c06@web.de>
On 12.12.16 19:12, Torsten Bögershausen wrote:
>
>>> Minor question, especially to the next commit:
>>> Should we make sure to checkout the exact version, which has been tested?
>>> In this case cb97792880625e24a9f581412d03659091a0e54f
>>>
>>> And this is for both a fresh clone and the git pull
>>> needs to be replaced by
>>> git fetch && git checkout cb97792880625e24a9f581412d03659091a0e54f
>>>
>>>
>>> (Which of course is a shell variable)
>>
>> I was actually wondering what the policy was for adding submodules to the Git repo,
>> but then decided against it. Another option would be to fork uniset on GitHub and
>> just let it stay on a working commit.
>>
>> Junio, what's your stance on this?
>>
>> Beat
>
> If I run ./update_unicode.sh on the latest master of https://github.com/depp/uniset.git ,
> commit a5fac4a091857dd5429cc2d, I get a diff in unicode_width.h like this:
>
> -{ 0x0300, 0x036F },
>
> +{ 768, 879 },
>
> IOW, all hex values are printed as decimal values.
> Not a problem for the compiler, but for the human
> to check the unicode tables.
>
> So I think we should "pin" the version of uniset.
That's what patch 3/3 fixes.
^ permalink raw reply
* Re: [PULL] minor git-svn updates (probably for 2.11.x)
From: Eric Wong @ 2016-12-12 19:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqmvg1m197.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> wrote:
> Thanks; basing these two on top of "Start post 2.11 cycle" that is
> on 'master' would mean that this won't merge to 'maint' for 2.11.x,
> though. I hope you wouldn't mind if I rebased them on top of
> 'maint' before merging?
Nope, wouldn't mind at all.
I was wondering if you preferred to cherry-pick them
individually since they're independent fixes.
^ permalink raw reply
* Subtree Split Bug - Irrelevant Commits Included in History
From: ELI @ 2016-12-12 19:12 UTC (permalink / raw)
To: git
Subtree Split has a bug in which irrelevant commits are included in
the sub-project's history. Along with breaking compatibility with
branches created before the bug was introduced, this significantly and
unnecessarily bloats the sub-project's git repository.
I contacted this group about a this bug back in April and later
followed up with a reproducible test case. However, I've failed to
get a response or traction, despite the significance of the bug.
My previous emails, including the reproduction steps, can be found in
the archives:
http://lists-archives.com/git/875747-subtree-split-includes-commits-outside-prefix-directory.html
Regards,
- Harpreet "Eli" Sangha
^ permalink raw reply
* Re: [PATCH] date-formats.txt: Typo fix
From: Junio C Hamano @ 2016-12-12 19:05 UTC (permalink / raw)
To: Luis Ressel; +Cc: git
In-Reply-To: <20161212164502.6187-1-aranea@aixah.de>
Luis Ressel <aranea@aixah.de> writes:
> Last time I checked, I was living in the UTC+01:00 time zone. UTC+02:00
> would be Central European _Summer_ Time.
>
> Signed-off-by: Luis Ressel <aranea@aixah.de>
> ---
> Documentation/date-formats.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks, will queue.
> diff --git a/Documentation/date-formats.txt b/Documentation/date-formats.txt
> index 35e8da201..6926e0a4c 100644
> --- a/Documentation/date-formats.txt
> +++ b/Documentation/date-formats.txt
> @@ -11,7 +11,7 @@ Git internal format::
> It is `<unix timestamp> <time zone offset>`, where `<unix
> timestamp>` is the number of seconds since the UNIX epoch.
> `<time zone offset>` is a positive or negative offset from UTC.
> - For example CET (which is 2 hours ahead UTC) is `+0200`.
> + For example CET (which is 1 hour ahead of UTC) is `+0100`.
>
> RFC 2822::
> The standard email format as described by RFC 2822, for example
^ permalink raw reply
* Re: [PATCH] git-svn: allow "0" in SVN path components
From: Junio C Hamano @ 2016-12-12 19:04 UTC (permalink / raw)
To: Eric Wong; +Cc: git
In-Reply-To: <20161212110914.GA24736@starla>
Eric Wong <e@80x24.org> writes:
> Blindly checking a path component for falsiness is unwise, as
> "0" is false to Perl, but a valid pathname component for SVN
> (or any filesystem).
>
> Found via random code reading.
>
> Signed-off-by: Eric Wong <e@80x24.org>
> ---
> Junio: this bugfix should go to "maint".
> Will push along with a doc fix for Juergen.
>
> perl/Git/SVN/Ra.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
> index e764696801..56ad9870bc 100644
> --- a/perl/Git/SVN/Ra.pm
> +++ b/perl/Git/SVN/Ra.pm
> @@ -606,7 +606,7 @@ sub minimize_url {
> my $latest = $ra->get_latest_revnum;
> $ra->get_log("", $latest, 0, 1, 0, 1, sub {});
> };
> - } while ($@ && ($c = shift @components));
> + } while ($@ && defined($c = shift @components));
>
> return canonicalize_url($url);
> }
Makes sense to me. Thanks.
^ permalink raw reply
* [PATCHv8 6/6] submodule: add absorb-git-dir function
From: Stefan Beller @ 2016-12-12 19:04 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161212190435.10358-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 absorbed
into the superprojects git directory.
The newly added code in this patch is structured such that other areas of
Git can also make use of it. The code in the submodule--helper is a mere
wrapper and option parser for the function
`absorb_git_dir_into_superproject`, that takes care of embedding the
submodules git directory into the superprojects git dir. That function
makes use of the more abstract function for this use case
`relocate_gitdir`, which can be used by e.g. the worktree code eventually
to move around a git directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/git-submodule.txt | 15 ++++++
builtin/submodule--helper.c | 38 ++++++++++++++
dir.c | 12 +++++
dir.h | 3 ++
git-submodule.sh | 7 ++-
submodule.c | 103 +++++++++++++++++++++++++++++++++++++
submodule.h | 4 ++
t/t7412-submodule-absorbgitdirs.sh | 101 ++++++++++++++++++++++++++++++++++++
8 files changed, 282 insertions(+), 1 deletion(-)
create mode 100755 t/t7412-submodule-absorbgitdirs.sh
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index d841573475..918bd1d1bd 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] absorbgitdirs [--] [<path>...]
DESCRIPTION
@@ -245,6 +246,20 @@ sync::
If `--recursive` is specified, this command will recurse into the
registered submodules, and sync any nested submodules within.
+absorbgitdirs::
+ If a git directory of a submodule is inside the submodule,
+ move the git directory of the submodule 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 embedded in the
+ superprojects git directory.
++
+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 5643848667..242d9911a6 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,6 +1076,43 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
return 0;
}
+static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
+{
+ int i;
+ struct pathspec pathspec;
+ struct module_list list = MODULE_LIST_INIT;
+ unsigned flags = ABSORB_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"),
+ ABSORB_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++)
+ absorb_git_dir_into_superproject(prefix,
+ list.entries[i]->name, flags);
+
+ return 0;
+}
+
#define SUPPORT_SUPER_PREFIX (1<<0)
struct cmd_struct {
@@ -1094,6 +1131,7 @@ static struct cmd_struct commands[] = {
{"resolve-relative-url-test", resolve_relative_url_test, 0},
{"init", module_init, 0},
{"remote-branch", resolve_remote_submodule_branch, 0},
+ {"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/dir.c b/dir.c
index e0efd3c2c3..d872cc1570 100644
--- a/dir.c
+++ b/dir.c
@@ -2773,3 +2773,15 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
free(work_tree);
free(git_dir);
}
+
+/*
+ * Migrate the git directory of the given path from old_git_dir to new_git_dir.
+ */
+void relocate_gitdir(const char *path, const char *old_git_dir, const char *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);
+}
diff --git a/dir.h b/dir.h
index 051674a431..bf23a470af 100644
--- a/dir.h
+++ b/dir.h
@@ -336,4 +336,7 @@ void write_untracked_extension(struct strbuf *out, struct untracked_cache *untra
void add_untracked_cache(struct index_state *istate);
void remove_untracked_cache(struct index_state *istate);
extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
+extern void relocate_gitdir(const char *path,
+ const char *old_git_dir,
+ const char *new_git_dir);
#endif
diff --git a/git-submodule.sh b/git-submodule.sh
index a024a135d6..9285b5c43d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1131,6 +1131,11 @@ cmd_sync()
done
}
+cmd_absorbgitdirs()
+{
+ git submodule--helper absorb-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 | absorbgitdirs)
command=$1
;;
-q|--quiet)
diff --git a/submodule.c b/submodule.c
index 0bb50b4b62..45ccfb7ab4 100644
--- a/submodule.c
+++ b/submodule.c
@@ -14,6 +14,7 @@
#include "blob.h"
#include "thread-utils.h"
#include "quote.h"
+#include "worktree.h"
static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
static int parallel_jobs = 1;
@@ -1237,3 +1238,105 @@ void prepare_submodule_repo_env(struct argv_array *out)
}
argv_array_push(out, "GIT_DIR=.git");
}
+
+/*
+ * Embeds a single submodules git directory into the superprojects git dir,
+ * non recursively.
+ */
+static void relocate_single_git_dir_into_superproject(const char *prefix,
+ const char *path)
+{
+ char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;
+ const char *new_git_dir;
+ const struct submodule *sub;
+
+ if (submodule_uses_worktrees(path))
+ die(_("relocate_gitdir for submodule '%s' with "
+ "more than one worktree not supported"), path);
+
+ old_git_dir = xstrfmt("%s/.git", path);
+ if (read_gitfile(old_git_dir))
+ /* If it is an actual gitfile, it doesn't need migration. */
+ return;
+
+ real_old_git_dir = xstrdup(real_path(old_git_dir));
+
+ 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);
+ real_new_git_dir = xstrdup(real_path(new_git_dir));
+
+ if (!prefix)
+ prefix = get_super_prefix();
+
+ fprintf(stderr, _("Migrating git directory of '%s%s' from\n'%s' to\n'%s'\n"),
+ prefix ? prefix : "", path,
+ real_old_git_dir, real_new_git_dir);
+
+ relocate_gitdir(path, real_old_git_dir, real_new_git_dir);
+
+ free(old_git_dir);
+ free(real_old_git_dir);
+ free(real_new_git_dir);
+}
+
+/*
+ * Migrate the git directory of the submodule given by path from
+ * having its git directory within the working tree to the git dir nested
+ * in its superprojects git dir under modules/.
+ */
+void absorb_git_dir_into_superproject(const char *prefix,
+ const char *path,
+ unsigned flags)
+{
+ const char *sub_git_dir, *v;
+ char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
+ struct strbuf gitdir = STRBUF_INIT;
+
+ strbuf_addf(&gitdir, "%s/.git", path);
+ sub_git_dir = resolve_gitdir(gitdir.buf);
+
+ /* Not populated? */
+ if (!sub_git_dir)
+ goto out;
+
+ /* Is it already absorbed into the superprojects git dir? */
+ real_sub_git_dir = xstrdup(real_path(sub_git_dir));
+ real_common_git_dir = xstrdup(real_path(get_git_common_dir()));
+ if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
+ relocate_single_git_dir_into_superproject(prefix, path);
+
+ if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (flags & ~ABSORB_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",
+ "absorb-git-dirs", NULL);
+ prepare_submodule_repo_env(&cp.env_array);
+ if (run_command(&cp))
+ die(_("could not recurse into submodule '%s'"), path);
+
+ strbuf_release(&sb);
+ }
+
+out:
+ strbuf_release(&gitdir);
+ free(real_sub_git_dir);
+ free(real_common_git_dir);
+}
diff --git a/submodule.h b/submodule.h
index 4e3bf469b4..6229054b99 100644
--- a/submodule.h
+++ b/submodule.h
@@ -74,4 +74,8 @@ int parallel_submodules(void);
*/
void prepare_submodule_repo_env(struct argv_array *out);
+#define ABSORB_GITDIR_RECURSE_SUBMODULES (1<<0)
+extern void absorb_git_dir_into_superproject(const char *prefix,
+ const char *path,
+ unsigned flags);
#endif
diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
new file mode 100755
index 0000000000..1c47780e2b
--- /dev/null
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -0,0 +1,101 @@
+#!/bin/sh
+
+test_description='Test submodule absorbgitdirs
+
+This test verifies that `git submodue absorbgitdirs` 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 'absorb 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 absorbgitdirs &&
+ 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 'absorbing does not fail for deinitalized submodules' '
+ test_when_finished "git submodule update --init" &&
+ git submodule deinit --all &&
+ git submodule absorbgitdirs &&
+ test -d .git/modules/sub1 &&
+ test -d sub1 &&
+ ! test -e sub1/.git
+'
+
+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 'absorb the git dir in a nested submodule' '
+ git status >expect.1 &&
+ git -C sub1/nested rev-parse HEAD >expect.2 &&
+ git submodule absorbgitdirs &&
+ 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 'absorbing the git dir fails for incomplete submodules' '
+ git status >expect.1 &&
+ git -C sub2 rev-parse HEAD >expect.2 &&
+ test_must_fail git submodule absorbgitdirs &&
+ 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 'absorbing fails for a submodule with multiple worktrees' '
+ test_must_fail git submodule absorbgitdirs sub3 2>error &&
+ test_i18ngrep "not supported" error
+'
+
+test_done
--
2.11.0.rc2.49.ge1f3b0c.dirty
^ permalink raw reply related
* [PATCHv8 3/6] test-lib-functions.sh: teach test_commit -C <dir>
From: Stefan Beller @ 2016-12-12 19:04 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161212190435.10358-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.11.0.rc2.49.ge1f3b0c.dirty
^ permalink raw reply related
* [PATCHv8 5/6] move connect_work_tree_and_git_dir to dir.h
From: Stefan Beller @ 2016-12-12 19:04 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161212190435.10358-1-sbeller@google.com>
That function was primarily used by submodule code, but the function
itself is not inherently about submodules. In the next patch we'll
introduce relocate_git_dir, which can be used by worktrees as well,
so find a neutral middle ground in dir.h.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
dir.c | 25 +++++++++++++++++++++++++
dir.h | 1 +
submodule.c | 25 -------------------------
submodule.h | 1 -
4 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/dir.c b/dir.c
index bfa8c8a9a5..e0efd3c2c3 100644
--- a/dir.c
+++ b/dir.c
@@ -2748,3 +2748,28 @@ void untracked_cache_add_to_index(struct index_state *istate,
{
untracked_cache_invalidate_path(istate, path);
}
+
+/* Update gitfile and core.worktree setting to connect work tree and git dir */
+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;
+ char *git_dir = xstrdup(real_path(git_dir_));
+ char *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, work_tree, &rel_path));
+
+ /* Update core.worktree setting */
+ strbuf_reset(&file_name);
+ strbuf_addf(&file_name, "%s/config", git_dir);
+ git_config_set_in_file(file_name.buf, "core.worktree",
+ relative_path(work_tree, git_dir, &rel_path));
+
+ strbuf_release(&file_name);
+ strbuf_release(&rel_path);
+ free(work_tree);
+ free(git_dir);
+}
diff --git a/dir.h b/dir.h
index 97c83bb383..051674a431 100644
--- a/dir.h
+++ b/dir.h
@@ -335,4 +335,5 @@ 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);
+extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
#endif
diff --git a/submodule.c b/submodule.c
index d4f7afe2f1..0bb50b4b62 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1222,31 +1222,6 @@ int merge_submodule(unsigned char result[20], const char *path,
return 0;
}
-/* Update gitfile and core.worktree setting to connect work tree and git dir */
-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;
- char *git_dir = xstrdup(real_path(git_dir_));
- char *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, work_tree, &rel_path));
-
- /* Update core.worktree setting */
- strbuf_reset(&file_name);
- strbuf_addf(&file_name, "%s/config", git_dir);
- git_config_set_in_file(file_name.buf, "core.worktree",
- relative_path(work_tree, git_dir, &rel_path));
-
- strbuf_release(&file_name);
- strbuf_release(&rel_path);
- free(work_tree);
- free(git_dir);
-}
-
int parallel_submodules(void)
{
return parallel_jobs;
diff --git a/submodule.h b/submodule.h
index d9e197a948..4e3bf469b4 100644
--- a/submodule.h
+++ b/submodule.h
@@ -65,7 +65,6 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned c
int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,
struct string_list *needs_pushing);
int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
-void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
int parallel_submodules(void);
/*
--
2.11.0.rc2.49.ge1f3b0c.dirty
^ permalink raw reply related
* [PATCHv8 4/6] worktree: check if a submodule uses worktrees
From: Stefan Beller @ 2016-12-12 19:04 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161212190435.10358-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
a submodule if it uses the worktree feature.
An earlier approach:
"Implement submodule_get_worktrees and just count them", however:
This can be done cheaply (both in new code to write as well as run time)
by obtaining the list of worktrees based off that submodules git
directory. However as we have loaded the variables for the current
repository, the values in the submodule worktree
can be wrong, e.g.
* core.ignorecase may differ between these two repositories
* the ref resolution is broken (refs/heads/branch in the submodule
resolves to the sha1 value of the `branch` in the current repository
that may not exist or have another sha1)
The implementation here is just checking for any files in
$GIT_COMMON_DIR/worktrees for the submodule, which ought to be sufficient
if the submodule is using the current repository format, which we also
check.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
worktree.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
worktree.h | 5 +++++
2 files changed, 55 insertions(+)
diff --git a/worktree.c b/worktree.c
index eb6121263b..d4606aa8cd 100644
--- a/worktree.c
+++ b/worktree.c
@@ -380,3 +380,53 @@ const struct worktree *find_shared_symref(const char *symref,
return existing;
}
+
+int submodule_uses_worktrees(const char *path)
+{
+ char *submodule_gitdir;
+ struct strbuf sb = STRBUF_INIT;
+ DIR *dir;
+ struct dirent *d;
+ int ret;
+ struct repository_format format;
+
+ submodule_gitdir = git_pathdup_submodule(path, "%s", "");
+ if (!submodule_gitdir)
+ return 0;
+
+ /* The env would be set for the superproject. */
+ get_common_dir_noenv(&sb, submodule_gitdir);
+
+ /*
+ * The check below is only known to be good for repository format
+ * version 0 at the time of writing this code.
+ */
+ strbuf_addstr(&sb, "/config");
+ read_repository_format(&format, sb.buf);
+ if (format.version != 0) {
+ strbuf_release(&sb);
+ return 1;
+ }
+
+ /* Replace config by worktrees. */
+ strbuf_setlen(&sb, sb.len - strlen("config"));
+ strbuf_addstr(&sb, "worktrees");
+
+ /* See if there is any file inside the worktrees directory. */
+ dir = opendir(sb.buf);
+ strbuf_release(&sb);
+ free(submodule_gitdir);
+
+ if (!dir)
+ return 0;
+
+ while ((d = readdir(dir)) != NULL) {
+ if (is_dot_or_dotdot(d->d_name))
+ continue;
+
+ ret = 1;
+ break;
+ }
+ closedir(dir);
+ return ret;
+}
diff --git a/worktree.h b/worktree.h
index d59ce1fee8..6bfb985203 100644
--- a/worktree.h
+++ b/worktree.h
@@ -27,6 +27,11 @@ struct worktree {
*/
extern struct worktree **get_worktrees(unsigned flags);
+/*
+ * Returns 1 if linked worktrees exist, 0 otherwise.
+ */
+extern int submodule_uses_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.11.0.rc2.49.ge1f3b0c.dirty
^ permalink raw reply related
* [PATCHv8 2/6] submodule helper: support super prefix
From: Stefan Beller @ 2016-12-12 19:04 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161212190435.10358-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>
---
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..5643848667 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 *);
+ unsigned 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.11.0.rc2.49.ge1f3b0c.dirty
^ permalink raw reply related
* [PATCHv8 1/6] submodule: use absolute path for computing relative path connecting
From: Stefan Beller @ 2016-12-12 19:04 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
In-Reply-To: <20161212190435.10358-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>
---
submodule.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/submodule.c b/submodule.c
index 6f7d883de9..d4f7afe2f1 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1223,27 +1223,28 @@ int merge_submodule(unsigned char result[20], const char *path,
}
/* Update gitfile and core.worktree setting to connect work tree and git dir */
-void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
+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 *git_dir = xstrdup(real_path(git_dir_));
+ char *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(git_dir, work_tree, &rel_path));
/* Update core.worktree setting */
strbuf_reset(&file_name);
strbuf_addf(&file_name, "%s/config", git_dir);
git_config_set_in_file(file_name.buf, "core.worktree",
- relative_path(real_work_tree, git_dir,
- &rel_path));
+ relative_path(work_tree, git_dir, &rel_path));
strbuf_release(&file_name);
strbuf_release(&rel_path);
- free((void *)real_work_tree);
+ free(work_tree);
+ free(git_dir);
}
int parallel_submodules(void)
--
2.11.0.rc2.49.ge1f3b0c.dirty
^ permalink raw reply related
* [PATCHv7 0/6] submodule absorbgitdirs
From: Stefan Beller @ 2016-12-12 19:04 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, pclouds, Stefan Beller
v8:
* Change the worktree implementation: do not have an internal
submodule_get_worktrees, and count the number of worktrees, instead
directly look if there is a worktrees dir and check if there are any files
in there.
* reworded one test title slightly.
* interdiff to v7 (that is queued as origin/sb/submodule-embed-gitdir) below.
v7:
* do not expose submodule_get_worktrees. The values may be wrong internally,
but for our purpose we do not care about the actual values, only the count.
Document the wrong values!
* more strings are _(marked up)
* renamed variables in connect_work_tree_and_git_dir
* unsigned instead of int for options in the submodule helper.
*
v6:
* renamed embedgitdirs to absorbgitdirs embedding may be interpreted as
embedding the git dir into the working directory, whereas absorbing sounds
more like the submodule is absorbed by the superproject, making the
submodule less independent
* Worktrees API offer uses_worktrees(void) and submodule_uses_worktree(path).
* moved the printing to stderr and one layer up (out of the pure
relocate_git_dir function).
* connect_... is in dir.h now.
v5:
* Add another layer of abstraction, i.e. the relocate_git_dir is only about
moving a git dir of one repository. The submodule specific stuff (e.g.
recursion into nested submodules) is in submodule.{c,h}
This was motivated by reviews on the series of checkout aware of submodules
building on top of this series, as we want to directly call the embed-git-dirs
function without the overhead of spawning a child process.
v4:
* rebuilt on top of nd/worktree-list-fixup
* fix and test behavior for un-init submodules (don't crash, rather do nothing)
* incorporated a "static" as pointed out by Ramsay
* use internal functions instead of duplicating code in worktree.c
(use get_common_dir_noenv for the submodule to actually get the common dir)
* fixed a memory leak in relocate_gitdir
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
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
Stefan Beller (6):
submodule: use absolute path for computing relative path connecting
submodule helper: support super prefix
test-lib-functions.sh: teach test_commit -C <dir>
worktree: have a function to check if worktrees are in use
move connect_work_tree_and_git_dir to dir.h
submodule: add absorb-git-dir function
Documentation/git-submodule.txt | 15 +++++
builtin/submodule--helper.c | 69 ++++++++++++++++----
dir.c | 37 +++++++++++
dir.h | 4 ++
git-submodule.sh | 7 +-
git.c | 2 +-
submodule.c | 127 ++++++++++++++++++++++++++++++-------
submodule.h | 5 +-
t/t7412-submodule-absorbgitdirs.sh | 101 +++++++++++++++++++++++++++++
t/test-lib-functions.sh | 20 ++++--
worktree.c | 68 +++++++++++++++++---
worktree.h | 7 ++
12 files changed, 409 insertions(+), 53 deletions(-)
create mode 100755 t/t7412-submodule-absorbgitdirs.sh
diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
index 7c4e8b8d79..1c47780e2b 100755
--- a/t/t7412-submodule-absorbgitdirs.sh
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -93,7 +93,7 @@ test_expect_success 'setup a submodule with multiple worktrees' '
git -C sub3 worktree add ../sub3_second_work_tree
'
-test_expect_success 'absorb a submodule with multiple worktrees' '
+test_expect_success 'absorbing fails for a submodule with multiple worktrees' '
test_must_fail git submodule absorbgitdirs sub3 2>error &&
test_i18ngrep "not supported" error
'
diff --git a/worktree.c b/worktree.c
index 1c46fcf25f..d4606aa8cd 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(const char *git_common_dir)
+static struct worktree *get_main_worktree(void)
{
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
@@ -81,12 +81,12 @@ static struct worktree *get_main_worktree(const char *git_common_dir)
int is_bare = 0;
int is_detached = 0;
- strbuf_add_absolute_path(&worktree_path, git_common_dir);
+ strbuf_add_absolute_path(&worktree_path, get_git_common_dir());
is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
if (is_bare)
strbuf_strip_suffix(&worktree_path, "/.");
- strbuf_addf(&path, "%s/HEAD", git_common_dir);
+ strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
@@ -101,8 +101,7 @@ static struct worktree *get_main_worktree(const char *git_common_dir)
return worktree;
}
-static struct worktree *get_linked_worktree(const char *git_common_dir,
- const char *id)
+static struct worktree *get_linked_worktree(const char *id)
{
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
@@ -113,7 +112,7 @@ static struct worktree *get_linked_worktree(const char *git_common_dir,
if (!id)
die("Missing linked worktree name");
- strbuf_addf(&path, "%s/worktrees/%s/gitdir", git_common_dir, id);
+ strbuf_git_common_path(&path, "worktrees/%s/gitdir", id);
if (strbuf_read_file(&worktree_path, path.buf, 0) <= 0)
/* invalid gitdir file */
goto done;
@@ -126,7 +125,7 @@ static struct worktree *get_linked_worktree(const char *git_common_dir,
}
strbuf_reset(&path);
- strbuf_addf(&path, "%s/worktrees/%s/HEAD", git_common_dir, id);
+ strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
@@ -168,8 +167,7 @@ static int compare_worktree(const void *a_, const void *b_)
return fspathcmp((*a)->path, (*b)->path);
}
-static struct worktree **get_worktrees_internal(const char *git_common_dir,
- unsigned flags)
+struct worktree **get_worktrees(unsigned flags)
{
struct worktree **list = NULL;
struct strbuf path = STRBUF_INIT;
@@ -179,10 +177,9 @@ static struct worktree **get_worktrees_internal(const char *git_common_dir,
list = xmalloc(alloc * sizeof(struct worktree *));
- if (!(flags & GWT_LINKED_ONLY))
- list[counter++] = get_main_worktree(git_common_dir);
+ list[counter++] = get_main_worktree();
- strbuf_addf(&path, "%s/worktrees", git_common_dir);
+ strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
dir = opendir(path.buf);
strbuf_release(&path);
if (dir) {
@@ -191,7 +188,7 @@ static struct worktree **get_worktrees_internal(const char *git_common_dir,
if (is_dot_or_dotdot(d->d_name))
continue;
- if ((linked = get_linked_worktree(git_common_dir, d->d_name))) {
+ if ((linked = get_linked_worktree(d->d_name))) {
ALLOC_GROW(list, counter + 1, alloc);
list[counter++] = linked;
}
@@ -212,34 +209,6 @@ static struct worktree **get_worktrees_internal(const char *git_common_dir,
return list;
}
-struct worktree **get_worktrees(unsigned flags)
-{
- return get_worktrees_internal(get_git_common_dir(), flags);
-}
-
-/*
- * NEEDSWORK: The values in the returned worktrees are broken, e.g.
- * the refs or path resolution is influenced by the current repository.
- */
-static struct worktree **get_submodule_worktrees(const char *path, unsigned flags)
-{
- char *submodule_gitdir;
- struct strbuf sb = STRBUF_INIT;
- struct worktree **ret;
-
- submodule_gitdir = git_pathdup_submodule(path, "%s", "");
- if (!submodule_gitdir)
- return NULL;
-
- /* the env would be set for the superproject */
- get_common_dir_noenv(&sb, submodule_gitdir);
- ret = get_worktrees_internal(sb.buf, flags);
-
- strbuf_release(&sb);
- free(submodule_gitdir);
- return ret;
-}
-
const char *get_worktree_git_dir(const struct worktree *wt)
{
if (!wt)
@@ -412,19 +381,52 @@ const struct worktree *find_shared_symref(const char *symref,
return existing;
}
-int uses_worktrees(void)
-{
- struct worktree **worktrees = get_worktrees(GWT_LINKED_ONLY);
- int retval = (worktrees != NULL && worktrees[0] != NULL);
- free_worktrees(worktrees);
- return retval;
-}
-
int submodule_uses_worktrees(const char *path)
{
- struct worktree **worktrees =
- get_submodule_worktrees(path, GWT_LINKED_ONLY);
- int retval = (worktrees != NULL && worktrees[0] != NULL);
- free_worktrees(worktrees);
- return retval;
+ char *submodule_gitdir;
+ struct strbuf sb = STRBUF_INIT;
+ DIR *dir;
+ struct dirent *d;
+ int ret;
+ struct repository_format format;
+
+ submodule_gitdir = git_pathdup_submodule(path, "%s", "");
+ if (!submodule_gitdir)
+ return 0;
+
+ /* The env would be set for the superproject. */
+ get_common_dir_noenv(&sb, submodule_gitdir);
+
+ /*
+ * The check below is only known to be good for repository format
+ * version 0 at the time of writing this code.
+ */
+ strbuf_addstr(&sb, "/config");
+ read_repository_format(&format, sb.buf);
+ if (format.version != 0) {
+ strbuf_release(&sb);
+ return 1;
+ }
+
+ /* Replace config by worktrees. */
+ strbuf_setlen(&sb, sb.len - strlen("config"));
+ strbuf_addstr(&sb, "worktrees");
+
+ /* See if there is any file inside the worktrees directory. */
+ dir = opendir(sb.buf);
+ strbuf_release(&sb);
+ free(submodule_gitdir);
+
+ if (!dir)
+ return 0;
+
+ while ((d = readdir(dir)) != NULL) {
+ if (is_dot_or_dotdot(d->d_name))
+ continue;
+
+ ret = 1;
+ break;
+ }
+ closedir(dir);
+ return ret;
}
diff --git a/worktree.h b/worktree.h
index ebe0856330..6bfb985203 100644
--- a/worktree.h
+++ b/worktree.h
@@ -16,7 +16,6 @@ struct worktree {
/* Functions for acting on the information about worktrees. */
#define GWT_SORT_LINKED (1 << 0) /* keeps linked worktrees sorted */
-#define GWT_LINKED_ONLY (1 << 1) /* do not include the main worktree */
/*
* Get the worktrees. The primary worktree will always be the first returned,
@@ -31,7 +30,6 @@ extern struct worktree **get_worktrees(unsigned flags);
/*
* Returns 1 if linked worktrees exist, 0 otherwise.
*/
-extern int uses_worktrees(void);
extern int submodule_uses_worktrees(const char *path);
/*
--
2.11.0.rc2.29.g7c00390.dirty
^ 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