* Re: [PATCHv5 0/5] submodule embedgitdirs
From: Junio C Hamano @ 2016-12-07 22:35 UTC (permalink / raw)
To: Stefan Beller; +Cc: bmwill, git, pclouds
In-Reply-To: <20161207210157.18932-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> 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.
OK. Comparing the last steps between this round and the previous
one, I do think the separation of the responsibility among helpers
is much more reasonable in this version, where:
- submodule_embed_git_dir() is given a single path and is
responsible for that submodule itself, which is done by calling
submodule_embed_git_dir_for_path() on itself, and its
sub-submodules, which is done by spawning the helper recursively
with appropriate super-prefix;
- submodule_embed_git_dir_for_path() computes where the given path
needs to be moved to using the knowledge specific to the
submodule subsystem, and asks relocate_gitdir() to perform the
actual relocation;
- relocate_gitdir() used to do quite a lot more, but now it is only
about moving an existing .git directory elsewhere and pointing to
the new location with .git file placed in the old location.
I would have called the second helper submodule_embed_one_git_dir(),
but that is a minor detail.
Very nicely done.
^ permalink raw reply
* Re: [PATCH] real_path: make real_path thread-safe
From: Brandon Williams @ 2016-12-07 22:29 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, git, sbeller, peff, jacob.keller
In-Reply-To: <7d968fd8-a92d-efd3-ce67-7de6049b6d56@kdbg.org>
On 12/07, Johannes Sixt wrote:
> Am 07.12.2016 um 01:10 schrieb Brandon Williams:
> >This function should accept both absolute and relative paths, which
> >means it should probably accept "C:\My Files". I wasn't thinking about
> >windows 100% of the time while writing this so I'm hoping that a windows
> >expert will point things like this out to me :).
>
> ;)
>
> With this patch, the test suite fails at the very first git init call:
>
> D:\Src\mingw-git\t>sh t0000-basic.sh -v -i -x
> fatal: Invalid path '/:': No such file or directory
> error: cannot run git init -- have you built things yet?
> FATAL: Unexpected exit with code 1
>
> I haven't dug further, yet.
>
> -- Hannes
>
Thanks for providing me with the error. Instead of assuming root is "/"
I'll need to extract what root is from an absolute path. Aside from
what root looks like, do most other path constructs behave similarly in
unix and windows? (like ".." and "." as examples)
Since I don't really have a windows machine to test things it might be
slightly difficult to get everything correct quickly but hopefully we can
get this working :)
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH] real_path: make real_path thread-safe
From: Brandon Williams @ 2016-12-07 22:13 UTC (permalink / raw)
To: Junio C Hamano
Cc: Torsten Bögershausen, Ramsay Jones, git, sbeller, peff,
jacob.keller
In-Reply-To: <xmqqtwafwkdt.fsf@gitster.mtv.corp.google.com>
On 12/07, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
> > But in any case it seems that e.g.
> > //SEFVER/SHARE/DIR1/DIR2/..
> > must be converted into
> > //SEFVER/SHARE/DIR1
> >
> > and
> > \\SEFVER\SHARE\DIR1\DIR2\..
> > must be converted into
> > \\SEFVER\SHARE\DIR1
>
> Additional questions that may be interesting are:
>
> //A/B/../C is it //A/C? is it an error?
> //A/B/../../C/D is it //C/D? is it an error?
>
Also is //.. the same as //? I would assume so since /.. is /
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH] submodule--helper: set alternateLocation for cloned submodules
From: Stefan Beller @ 2016-12-07 22:09 UTC (permalink / raw)
To: vi0oss; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <9493435c-16e5-d8af-9455-ec3f3cba390d@gmail.com>
On Wed, Dec 7, 2016 at 1:24 PM, vi0oss <vi0oss@gmail.com> wrote:
> On 12/07/2016 11:09 PM, Stefan Beller wrote:
>>>
>>> As submodule's alternate target does not end in .git/objects
>>> (rather .git/modules/qqqqqq/objects), this alternate target
>>> path restriction for in add_possible_reference_from_superproject
>>> relates from "*.git/objects" to just */objects".
>>
>> I wonder if this check is too weak and we actually have to check for
>> either .git/objects or modules/<name/possibly/having/slashes>/objects.
>> When writing the referenced commit I assumed we'd need a stronger check
>> to be safer and not add some random location as a possible alternate.
>>
> 1. Do we really need to check that it is named ".git"? Although
> "git clone --mirror --recursive" is not (directly) supported
> now, user may create one bare repository with [remnants of]
> submodules by converting reqular repository into bare one.
> Why not take advantage of additional information available locally
> in this case?
Oh, great point.
>
> 2. Is the check need to be strict because of we need to traverse
> one directory level up? Normally this "/objects" part is added by
> Git, so just one "../" seems to be OK. User can't specify "--reference
> somerepo/.git/objects", a strange reference can appear only if user
> manually creates alternates. Maybe better to document this case
> instead of restricting the feature?
Not sure I understand what needs better documentation here?
>
> 3. If nonetheless check for ".git/*/objects", then
> a. What functions should be used in Git codebase for such checks?
> b. Should we handle tricks like "smth/.git/../../objects" and so on?
I see we're getting into problems here.
>
> 4. Should we print (or print in verbose mode) each used alternate,
> to inform operator what his or her new clone will depend on?
>
> P.S. Actually I discovered the --recursive --reference feature when tried to
> put reference to a mega-repo with all possible submodules added as remotes.
> I expected --reference to just get though across all submodules, but it
> complained
> to missing "/modules/..." instead (the check went though becase the
> repository
> was named like "megarepo.git", so it did ended in ".git/objects").
Oh :(
With that said, I think the original patch is a sensible approach.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 2/2] describe: add support for multiple match patterns
From: Junio C Hamano @ 2016-12-07 22:08 UTC (permalink / raw)
To: Jacob Keller; +Cc: git, Jacob Keller
In-Reply-To: <20161207023259.29355-2-jacob.e.keller@intel.com>
Jacob Keller <jacob.e.keller@intel.com> writes:
> ... Suppose that you version all
> your official releases such as "v1.2", "v1.3", "v1.4", "v2.1" and so on.
> Now, you also have other tags which represent -rc releases and other
> such tags. If you want to find the first major release that contains
> a given commit you might try
>
> git describe --contains --match="v?.?" <commit>
>
> This will work as long as you have only single digits. But if you start
> adding multiple digits, the pattern becomes not enough to match all the
> tags you wanted while excluding the ones you didn't.
Isn't what you really want for the use case a negative pattern,
i.e. "I want ones that match v* but not the ones that match *-rc*",
though?
^ permalink raw reply
* Re: [PATCH 1/3] wt-status: implement opportunisitc index update correctly
From: Stefan Beller @ 2016-12-07 22:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Paul Tan
In-Reply-To: <xmqqmvg7wips.fsf@gitster.mtv.corp.google.com>
On Wed, Dec 7, 2016 at 1:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> So my first question I had to answer was if we do the right thing here,
>> i.e. if we could just fail instead. But we want to continue and just
>> not write back the index, which is fine.
>>
>> So we do not have to guard refresh_cache, but just call
>> update_index_if_able conditionally.
>
> An explanation with stepping back a little bit may help.
>
> You may be asked to visit a repository of a friend, to which you do
> not have write access to but you can still read. You may want to do
> "diff", "status" or "describe" there.
>
> In order to avoid getting fooled into thinking some paths are dirty
> only because the cached stat information does not match, these need
> to refresh the in-core index before doing their "comparison" to
> report which paths are different (in "diff"), what are the modified
> but not staged paths (in "status"), and if there is a need to add
> the "-dirty" suffix (in "describe").
>
> Since we are doing the expensive "bunch of lstat()" anyway, if we
> could write it back to the index, it would help future operations in
> the same repository--that is the reasoning behind the opportunistic
> updates. It is perfectly OK if we do not have write access to the
> repository and cannot write update the index.
>
Thanks for that explanation!
So I agree that we should not call it _if_needed, but the _if_able
part is still confusing, as we check for different parts here.
The case of not being able to write (read only) sounds similar to
the case of not being able to write (a concurrent lock),
I'll think about it more.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 00/17] pathspec cleanup
From: Junio C Hamano @ 2016-12-07 22:04 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller, pclouds
In-Reply-To: <1481061106-117775-1-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> The intent of this series is to cleanup some of the pathspec initialization
> code as well as finally migrating the remaining users of the _raw field or
> get_pathspec() to the pathspec struct interface. This way both the _raw field
> and get_pathspec() can be removed from the codebase. This also removes the
> functionality where parse_pathspec() modified the const char * argv array that
> was passed in (which felt kind of odd to me as I wouldn't have expected the
> passed in array to be modified).
>
> I also noticed that there are memory leaks associated with the 'original' and
> 'match' strings. To fix this the pathspec struct needed to take ownership of
> the memory for these fields so that they can be cleaned up when clearing the
> pathspec struct.
Both good goals. Thanks for working on this.
^ permalink raw reply
* Re: [PATCH 16/17] pathspec: small readability changes
From: Junio C Hamano @ 2016-12-07 22:04 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Brandon Williams, Git Mailing List, Stefan Beller
In-Reply-To: <CACsJy8B6Mj-L1t-CETY5DWRyABHZsYZszwXD3dgUqChfXRB6FA@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
>> A few small changes to improve readability. This is done by grouping related
>> assignments, adding blank lines, ensuring lines are <80 characters, etc.
>>
>> Signed-off-by: Brandon Williams <bmwill@google.com>
>> ---
>> pathspec.c | 15 ++++++++++-----
>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/pathspec.c b/pathspec.c
>> index 41aa213..8a07b02 100644
>> --- a/pathspec.c
>> +++ b/pathspec.c
>> @@ -334,6 +334,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>
> btw, since this function has stopped being "just prefix pathspec" for
> a long time, perhaps rename it to parse_pathspec_item, or something.
Not specifically responding to this comment, but thanks for all the
constructive feedback messages.
^ permalink raw reply
* [PATCH 5/5] sequencer: Remove useless get_dir() function
From: Stephan Beyer @ 2016-12-07 21:51 UTC (permalink / raw)
To: git; +Cc: Stephan Beyer, Christian Couder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <xmqqlgvs28bh.fsf@gitster.mtv.corp.google.com>
This function is used only once, for the removal of the
directory. It is not used for the creation of the directory
nor anywhere else.
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
sequencer.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index c9b560ac1..689cfa5f1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -47,11 +47,6 @@ static inline int is_rebase_i(const struct replay_opts *opts)
return 0;
}
-static const char *get_dir(const struct replay_opts *opts)
-{
- return git_path_seq_dir();
-}
-
static const char *get_todo_path(const struct replay_opts *opts)
{
return git_path_todo_file();
@@ -160,7 +155,7 @@ int sequencer_remove_state(struct replay_opts *opts)
free(opts->xopts[i]);
free(opts->xopts);
- strbuf_addf(&dir, "%s", get_dir(opts));
+ strbuf_addf(&dir, "%s", git_path_seq_dir());
remove_dir_recursively(&dir, 0);
strbuf_release(&dir);
--
2.11.0.27.g4eed97c
^ permalink raw reply related
* [PATCH 3/5] Add test that cherry-pick --abort does not unsafely change HEAD
From: Stephan Beyer @ 2016-12-07 21:51 UTC (permalink / raw)
To: git; +Cc: Stephan Beyer, Christian Couder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <xmqqlgvs28bh.fsf@gitster.mtv.corp.google.com>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
t/t3510-cherry-pick-sequence.sh | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 7b7a89dbd..372307c21 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -147,6 +147,16 @@ test_expect_success '--abort to cancel single cherry-pick' '
git diff-index --exit-code HEAD
'
+test_expect_success '--abort does not unsafely change HEAD' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick picked anotherpick &&
+ git reset --hard base &&
+ test_must_fail git cherry-pick picked anotherpick &&
+ git cherry-pick --abort 2>actual &&
+ test_i18ngrep "You seem to have moved HEAD" actual &&
+ test_cmp_rev base HEAD
+'
+
test_expect_success 'cherry-pick --abort to cancel multiple revert' '
pristine_detach anotherpick &&
test_expect_code 1 git revert base..picked &&
--
2.11.0.27.g4eed97c
^ permalink raw reply related
* [PATCH 1/5] am: Fix filename in safe_to_abort() error message
From: Stephan Beyer @ 2016-12-07 21:51 UTC (permalink / raw)
To: git; +Cc: Stephan Beyer, Christian Couder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <xmqqlgvs28bh.fsf@gitster.mtv.corp.google.com>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
Okay let's give it a try. Some minor things that I found
are also in this patchset (patch 01, 02 and 05).
The branch can also be found on
https://github.com/sbeyer/git/commits/sequencer-abort-safety
builtin/am.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/am.c b/builtin/am.c
index 6981f42ce..7cf40e6f2 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2124,7 +2124,7 @@ static int safe_to_abort(const struct am_state *state)
if (read_state_file(&sb, state, "abort-safety", 1) > 0) {
if (get_oid_hex(sb.buf, &abort_safety))
- die(_("could not parse %s"), am_path(state, "abort_safety"));
+ die(_("could not parse %s"), am_path(state, "abort-safety"));
} else
oidclr(&abort_safety);
--
2.11.0.27.g4eed97c
^ permalink raw reply related
* [PATCH 2/5] am: Change safe_to_abort()'s not rewinding error into a warning
From: Stephan Beyer @ 2016-12-07 21:51 UTC (permalink / raw)
To: git; +Cc: Stephan Beyer, Christian Couder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <xmqqlgvs28bh.fsf@gitster.mtv.corp.google.com>
The error message tells the user that something went terribly wrong
and the --abort could not be performed. But the --abort is performed,
only without rewinding. By simply changing the error into a warning,
we indicate the user that she must not try something like
"git am --abort --force", instead she just has to check the HEAD.
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
builtin/am.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/am.c b/builtin/am.c
index 7cf40e6f2..826f18ba1 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2134,7 +2134,7 @@ static int safe_to_abort(const struct am_state *state)
if (!oidcmp(&head, &abort_safety))
return 1;
- error(_("You seem to have moved HEAD since the last 'am' failure.\n"
+ warning(_("You seem to have moved HEAD since the last 'am' failure.\n"
"Not rewinding to ORIG_HEAD"));
return 0;
--
2.11.0.27.g4eed97c
^ permalink raw reply related
* [PATCH 4/5] Make sequencer abort safer
From: Stephan Beyer @ 2016-12-07 21:51 UTC (permalink / raw)
To: git; +Cc: Stephan Beyer, Christian Couder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <xmqqlgvs28bh.fsf@gitster.mtv.corp.google.com>
In contrast to "git am --abort", a sequencer abort did not check
whether the current HEAD is the one that is expected. This can
lead to loss of work (when not spotted and resolved using reflog
before the garbage collector chimes in).
This behavior is now changed by mimicking "git am --abort":
the abortion is done but HEAD is not changed when the current HEAD
is not the expected HEAD.
A new file "sequencer/current" is added to save the expected HEAD.
The new behavior is only active when --abort is invoked on multiple
picks. The problem does not occur for the single-pick case because
it is handled differently.
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
sequencer.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index 30b10ba14..c9b560ac1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -27,6 +27,7 @@ GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
+static GIT_PATH_FUNC(git_path_curr_file, "sequencer/current")
/*
* A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
@@ -310,6 +311,20 @@ static int error_dirty_index(struct replay_opts *opts)
return -1;
}
+static void update_curr_file()
+{
+ struct object_id head;
+
+ /* Do nothing on a single-pick */
+ if (!file_exists(git_path_seq_dir()))
+ return;
+
+ if (!get_oid("HEAD", &head))
+ write_file(git_path_curr_file(), "%s", oid_to_hex(&head));
+ else
+ write_file(git_path_curr_file(), "%s", "");
+}
+
static int fast_forward_to(const unsigned char *to, const unsigned char *from,
int unborn, struct replay_opts *opts)
{
@@ -339,6 +354,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,
strbuf_release(&sb);
strbuf_release(&err);
ref_transaction_free(transaction);
+ update_curr_file();
return 0;
}
@@ -813,6 +829,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
leave:
free_message(commit, &msg);
+ update_curr_file();
return res;
}
@@ -1132,9 +1149,34 @@ static int save_head(const char *head)
return 0;
}
+static int rollback_is_safe()
+{
+ struct strbuf sb = STRBUF_INIT;
+ struct object_id expected_head, actual_head;
+
+ if (strbuf_read_file(&sb, git_path_curr_file(), 0) >= 0) {
+ strbuf_trim(&sb);
+ if (get_oid_hex(sb.buf, &expected_head)) {
+ strbuf_release(&sb);
+ die(_("could not parse %s"), git_path_curr_file());
+ }
+ strbuf_release(&sb);
+ }
+ else if (errno == ENOENT)
+ oidclr(&expected_head);
+ else
+ die_errno(_("could not read '%s'"), git_path_curr_file());
+
+ if (get_oid("HEAD", &actual_head))
+ oidclr(&actual_head);
+
+ return !oidcmp(&actual_head, &expected_head);
+}
+
static int reset_for_rollback(const unsigned char *sha1)
{
const char *argv[4]; /* reset --merge <arg> + NULL */
+
argv[0] = "reset";
argv[1] = "--merge";
argv[2] = sha1_to_hex(sha1);
@@ -1189,6 +1231,12 @@ int sequencer_rollback(struct replay_opts *opts)
error(_("cannot abort from a branch yet to be born"));
goto fail;
}
+
+ if (!rollback_is_safe()) {
+ /* Do not error, just do not rollback */
+ warning(_("You seem to have moved HEAD. "
+ "Not rewinding, check your HEAD!"));
+ } else
if (reset_for_rollback(sha1))
goto fail;
strbuf_release(&buf);
@@ -1393,6 +1441,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return -1;
if (save_opts(opts))
return -1;
+ update_curr_file();
res = pick_commits(&todo_list, opts);
todo_list_release(&todo_list);
return res;
--
2.11.0.27.g4eed97c
^ permalink raw reply related
* Re: [PATCH] submodule--helper: set alternateLocation for cloned submodules
From: vi0oss @ 2016-12-07 21:24 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <CAGZ79kY3LR2KA69b4iDJb164EhJLb3JuVSRRcN0-4-kp-eryog@mail.gmail.com>
On 12/07/2016 11:09 PM, Stefan Beller wrote:
>> As submodule's alternate target does not end in .git/objects
>> (rather .git/modules/qqqqqq/objects), this alternate target
>> path restriction for in add_possible_reference_from_superproject
>> relates from "*.git/objects" to just */objects".
> I wonder if this check is too weak and we actually have to check for
> either .git/objects or modules/<name/possibly/having/slashes>/objects.
> When writing the referenced commit I assumed we'd need a stronger check
> to be safer and not add some random location as a possible alternate.
>
1. Do we really need to check that it is named ".git"? Although
"git clone --mirror --recursive" is not (directly) supported
now, user may create one bare repository with [remnants of]
submodules by converting reqular repository into bare one.
Why not take advantage of additional information available locally
in this case?
2. Is the check need to be strict because of we need to traverse
one directory level up? Normally this "/objects" part is added by
Git, so just one "../" seems to be OK. User can't specify "--reference
somerepo/.git/objects", a strange reference can appear only if user
manually creates alternates. Maybe better to document this case
instead of restricting the feature?
3. If nonetheless check for ".git/*/objects", then
a. What functions should be used in Git codebase for such checks?
b. Should we handle tricks like "smth/.git/../../objects" and so on?
4. Should we print (or print in verbose mode) each used alternate,
to inform operator what his or her new clone will depend on?
P.S. Actually I discovered the --recursive --reference feature when tried to
put reference to a mega-repo with all possible submodules added as remotes.
I expected --reference to just get though across all submodules, but it
complained
to missing "/modules/..." instead (the check went though becase the
repository
was named like "megarepo.git", so it did ended in ".git/objects").
^ permalink raw reply
* Re: [PATCH 1/3] wt-status: implement opportunisitc index update correctly
From: Junio C Hamano @ 2016-12-07 21:08 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Paul Tan
In-Reply-To: <CAGZ79kZHGqU2y19_uKhtVuE6vhspzPNpw-nVDnm8gLQ8u528kQ@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> So my first question I had to answer was if we do the right thing here,
> i.e. if we could just fail instead. But we want to continue and just
> not write back the index, which is fine.
>
> So we do not have to guard refresh_cache, but just call
> update_index_if_able conditionally.
An explanation with stepping back a little bit may help.
You may be asked to visit a repository of a friend, to which you do
not have write access to but you can still read. You may want to do
"diff", "status" or "describe" there.
In order to avoid getting fooled into thinking some paths are dirty
only because the cached stat information does not match, these need
to refresh the in-core index before doing their "comparison" to
report which paths are different (in "diff"), what are the modified
but not staged paths (in "status"), and if there is a need to add
the "-dirty" suffix (in "describe").
Since we are doing the expensive "bunch of lstat()" anyway, if we
could write it back to the index, it would help future operations in
the same repository--that is the reasoning behind the opportunistic
updates. It is perfectly OK if we do not have write access to the
repository and cannot write update the index.
^ permalink raw reply
* [PATCHv5 1/5] submodule: use absolute path for computing relative path connecting
From: Stefan Beller @ 2016-12-07 21:01 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161207210157.18932-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.11.0.rc2.28.g2af45f1.dirty
^ permalink raw reply related
* [PATCHv5 5/5] submodule: add embed-git-dir function
From: Stefan Beller @ 2016-12-07 21:01 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161207210157.18932-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.
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 `submodule_embed_git_dir_for_path`, 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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-submodule.txt | 14 +++++
builtin/submodule--helper.c | 39 ++++++++++++-
dir.c | 27 +++++++++
dir.h | 3 +
git-submodule.sh | 7 ++-
submodule.c | 115 ++++++++++++++++++++++++++++++++++++++
submodule.h | 7 +++
t/t7412-submodule-embedgitdirs.sh | 101 +++++++++++++++++++++++++++++++++
8 files changed, 311 insertions(+), 2 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..321c9e250a 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++)
+ submodule_embed_git_dir(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..e023b04407 100644
--- a/dir.c
+++ b/dir.c
@@ -2748,3 +2748,30 @@ void untracked_cache_add_to_index(struct index_state *istate,
{
untracked_cache_invalidate_path(istate, path);
}
+
+/*
+ * Migrate the git directory of the given `path` from `old_git_dir` to
+ * `new_git_dir`. If an error occurs, append it to `err` and return the
+ * error code.
+ */
+int relocate_gitdir(const char *path, const char *old_git_dir,
+ const char *new_git_dir, const char *displaypath,
+ struct strbuf *err)
+{
+ int ret = 0;
+
+ printf("Migrating git directory of '%s' from\n'%s' to\n'%s'\n",
+ displaypath, old_git_dir, new_git_dir);
+
+ if (rename(old_git_dir, new_git_dir) < 0) {
+ ret = errno;
+ strbuf_addf(err,
+ _("could not migrate git directory from '%s' to '%s'"),
+ old_git_dir, new_git_dir);
+ return ret;
+ }
+
+ connect_work_tree_and_git_dir(path, new_git_dir);
+
+ return ret;
+}
diff --git a/dir.h b/dir.h
index 97c83bb383..bf06729a86 100644
--- a/dir.h
+++ b/dir.h
@@ -335,4 +335,7 @@ 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 int relocate_gitdir(const char *path, const char *old_git_dir,
+ const char *new_git_dir, const char *displaypath,
+ struct strbuf *err);
#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.c b/submodule.c
index 66c5ce5a24..67a91275b8 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;
@@ -1263,3 +1264,117 @@ void prepare_submodule_repo_env(struct argv_array *out)
}
argv_array_push(out, "GIT_DIR=.git");
}
+
+/* Embeds a single submodule, non recursively. */
+static void submodule_embed_git_dir_for_path(const char *prefix, const char *path)
+{
+ struct worktree **worktrees;
+ struct strbuf pathbuf = STRBUF_INIT;
+ struct strbuf errbuf = STRBUF_INIT;
+ char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;
+ const char *new_git_dir;
+ const struct submodule *sub;
+ int code;
+
+ worktrees = get_submodule_worktrees(path, 0);
+ if (worktrees) {
+ int i;
+ for (i = 0; worktrees[i]; i++)
+ ;
+ free_worktrees(worktrees);
+ if (i > 1)
+ 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();
+ strbuf_addf(&pathbuf, "%s%s", prefix ? prefix : "", path);
+
+ code = relocate_gitdir(path, real_old_git_dir, real_new_git_dir,
+ pathbuf.buf, &errbuf);
+ if (code) {
+ errno = code;
+ die_errno("%s\n", errbuf.buf);
+ }
+
+ free(old_git_dir);
+ free(real_old_git_dir);
+ free(real_new_git_dir);
+ strbuf_release(&pathbuf);
+}
+
+/*
+ * 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/.
+ */
+int submodule_embed_git_dir(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 embedded? */
+ 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))
+ submodule_embed_git_dir_for_path(prefix, path);
+
+ 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 recurse into submodule '%s'"), path);
+
+ strbuf_release(&sb);
+ }
+
+out:
+ strbuf_release(&gitdir);
+ free(real_sub_git_dir);
+ free(real_common_git_dir);
+ return 0;
+}
diff --git a/submodule.h b/submodule.h
index d9e197a948..922cfd258f 100644
--- a/submodule.h
+++ b/submodule.h
@@ -75,4 +75,11 @@ int parallel_submodules(void);
*/
void prepare_submodule_repo_env(struct argv_array *out);
+/*
+ * Embed a git dir of the submodule given by path.
+ */
+#define RELOCATE_GITDIR_RECURSE_SUBMODULES (1<<0)
+extern int submodule_embed_git_dir(const char *prefix,
+ const char *path,
+ unsigned flags);
#endif
diff --git a/t/t7412-submodule-embedgitdirs.sh b/t/t7412-submodule-embedgitdirs.sh
new file mode 100755
index 0000000000..a02e7c447a
--- /dev/null
+++ b/t/t7412-submodule-embedgitdirs.sh
@@ -0,0 +1,101 @@
+#!/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 'embedding does not fail for deinitalized submodules' '
+ test_when_finished "git submodule update --init" &&
+ git submodule deinit --all &&
+ git submodule embedgitdirs &&
+ test -d .git/modules/sub1 &&
+ ! test -f sub1/.git &&
+ test -d sub1
+'
+
+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.11.0.rc2.28.g2af45f1.dirty
^ permalink raw reply related
* [PATCHv5 4/5] worktree: get worktrees from submodules
From: Stefan Beller @ 2016-12-07 21:01 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161207210157.18932-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 | 47 +++++++++++++++++++++++++++++++++++++----------
worktree.h | 6 ++++++
2 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/worktree.c b/worktree.c
index eb6121263b..fa2b6dfa9a 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);
worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
@@ -101,7 +101,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;
@@ -112,7 +113,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;
@@ -125,7 +126,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;
@@ -167,7 +168,8 @@ static int compare_worktree(const void *a_, const void *b_)
return fspathcmp((*a)->path, (*b)->path);
}
-struct worktree **get_worktrees(unsigned flags)
+static struct worktree **get_worktrees_internal(const char *git_common_dir,
+ unsigned flags)
{
struct worktree **list = NULL;
struct strbuf path = STRBUF_INIT;
@@ -177,9 +179,9 @@ struct worktree **get_worktrees(unsigned flags)
list = xmalloc(alloc * sizeof(struct worktree *));
- list[counter++] = get_main_worktree();
+ list[counter++] = get_main_worktree(git_common_dir);
- 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) {
@@ -188,7 +190,7 @@ struct worktree **get_worktrees(unsigned flags)
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 +211,31 @@ struct worktree **get_worktrees(unsigned flags)
return list;
}
+struct worktree **get_worktrees(unsigned flags)
+{
+ return get_worktrees_internal(get_git_common_dir(), flags);
+}
+
+struct worktree **get_submodule_worktrees(const char *path, unsigned flags)
+{
+ char *submodule_gitdir;
+ const char *submodule_common_dir;
+ 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);
+ submodule_common_dir = strbuf_detach(&sb, NULL);
+ ret = get_worktrees_internal(submodule_common_dir, flags);
+
+ free(submodule_gitdir);
+ return ret;
+}
+
const char *get_worktree_git_dir(const struct worktree *wt)
{
if (!wt)
diff --git a/worktree.h b/worktree.h
index d59ce1fee8..157fbc4a66 100644
--- a/worktree.h
+++ b/worktree.h
@@ -27,6 +27,12 @@ struct worktree {
*/
extern struct worktree **get_worktrees(unsigned flags);
+/*
+ * Get the worktrees of a submodule named by `path`.
+ */
+extern struct worktree **get_submodule_worktrees(const char *path,
+ unsigned flags);
+
/*
* 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.28.g2af45f1.dirty
^ permalink raw reply related
* [PATCHv5 3/5] test-lib-functions.sh: teach test_commit -C <dir>
From: Stefan Beller @ 2016-12-07 21:01 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161207210157.18932-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.28.g2af45f1.dirty
^ permalink raw reply related
* [PATCHv5 2/5] submodule helper: support super prefix
From: Stefan Beller @ 2016-12-07 21:01 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
In-Reply-To: <20161207210157.18932-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.11.0.rc2.28.g2af45f1.dirty
^ permalink raw reply related
* [PATCHv5 0/5] submodule embedgitdirs
From: Stefan Beller @ 2016-12-07 21:01 UTC (permalink / raw)
To: bmwill; +Cc: git, pclouds, gitster, Stefan Beller
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.
Thanks,
Stefan
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 (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 | 27 ++++++++
dir.h | 3 +
git-submodule.sh | 7 ++-
git.c | 2 +-
submodule.c | 127 ++++++++++++++++++++++++++++++++++++--
submodule.h | 7 +++
t/t7412-submodule-embedgitdirs.sh | 101 ++++++++++++++++++++++++++++++
t/test-lib-functions.sh | 20 ++++--
worktree.c | 47 +++++++++++---
worktree.h | 6 ++
12 files changed, 396 insertions(+), 33 deletions(-)
create mode 100755 t/t7412-submodule-embedgitdirs.sh
diff to v4:
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 10df69c86a..321c9e250a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1106,31 +1106,8 @@ static int embed_git_dir(int argc, const char **argv, const char *prefix)
if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
return 1;
- for (i = 0; i < list.nr; i++) {
- const char *path = list.entries[i]->name, *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 free_and_continue;
-
- /* Is it already embedded? */
- 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), NULL)
- goto free_and_continue;
-
- relocate_gitdir(prefix, path, flags);
-
-free_and_continue:
- strbuf_release(&gitdir);
- free(real_sub_git_dir);
- free(real_common_git_dir);
- }
+ for (i = 0; i < list.nr; i++)
+ submodule_embed_git_dir(prefix, list.entries[i]->name, flags);
return 0;
}
diff --git a/dir.c b/dir.c
index d2f60b5abf..e023b04407 100644
--- a/dir.c
+++ b/dir.c
@@ -15,9 +15,6 @@
#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;
@@ -2753,79 +2750,28 @@ void untracked_cache_add_to_index(struct index_state *istate,
}
/*
- * 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/.
+ * Migrate the git directory of the given `path` from `old_git_dir` to
+ * `new_git_dir`. If an error occurs, append it to `err` and return the
+ * error code.
*/
-void relocate_gitdir(const char *prefix, const char *path, unsigned flags)
+int relocate_gitdir(const char *path, const char *old_git_dir,
+ const char *new_git_dir, const char *displaypath,
+ struct strbuf *err)
{
- char *old_git_dir;
- const char *new_git_dir;
- const struct submodule *sub;
- struct worktree **worktrees;
- int i;
-
- worktrees = get_submodule_worktrees(path, 0);
- if (worktrees) {
- for (i = 0; worktrees[i]; i++)
- ;
- free_worktrees(worktrees);
- 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);
+ int ret = 0;
- 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);
+ printf("Migrating git directory of '%s' from\n'%s' to\n'%s'\n",
+ displaypath, old_git_dir, 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'"),
+ if (rename(old_git_dir, new_git_dir) < 0) {
+ ret = errno;
+ strbuf_addf(err,
+ _("could not migrate git directory from '%s' to '%s'"),
old_git_dir, new_git_dir);
+ return ret;
+ }
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);
+ return ret;
}
diff --git a/dir.h b/dir.h
index 0b5e99b21d..bf06729a86 100644
--- a/dir.h
+++ b/dir.h
@@ -335,8 +335,7 @@ 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);
-
+extern int relocate_gitdir(const char *path, const char *old_git_dir,
+ const char *new_git_dir, const char *displaypath,
+ struct strbuf *err);
#endif
diff --git a/submodule.c b/submodule.c
index 66c5ce5a24..67a91275b8 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;
@@ -1263,3 +1264,117 @@ void prepare_submodule_repo_env(struct argv_array *out)
}
argv_array_push(out, "GIT_DIR=.git");
}
+
+/* Embeds a single submodule, non recursively. */
+static void submodule_embed_git_dir_for_path(const char *prefix, const char *path)
+{
+ struct worktree **worktrees;
+ struct strbuf pathbuf = STRBUF_INIT;
+ struct strbuf errbuf = STRBUF_INIT;
+ char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;
+ const char *new_git_dir;
+ const struct submodule *sub;
+ int code;
+
+ worktrees = get_submodule_worktrees(path, 0);
+ if (worktrees) {
+ int i;
+ for (i = 0; worktrees[i]; i++)
+ ;
+ free_worktrees(worktrees);
+ if (i > 1)
+ 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();
+ strbuf_addf(&pathbuf, "%s%s", prefix ? prefix : "", path);
+
+ code = relocate_gitdir(path, real_old_git_dir, real_new_git_dir,
+ pathbuf.buf, &errbuf);
+ if (code) {
+ errno = code;
+ die_errno("%s\n", errbuf.buf);
+ }
+
+ free(old_git_dir);
+ free(real_old_git_dir);
+ free(real_new_git_dir);
+ strbuf_release(&pathbuf);
+}
+
+/*
+ * 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/.
+ */
+int submodule_embed_git_dir(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 embedded? */
+ 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))
+ submodule_embed_git_dir_for_path(prefix, path);
+
+ 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 recurse into submodule '%s'"), path);
+
+ strbuf_release(&sb);
+ }
+
+out:
+ strbuf_release(&gitdir);
+ free(real_sub_git_dir);
+ free(real_common_git_dir);
+ return 0;
+}
diff --git a/submodule.h b/submodule.h
index d9e197a948..922cfd258f 100644
--- a/submodule.h
+++ b/submodule.h
@@ -75,4 +75,11 @@ int parallel_submodules(void);
*/
void prepare_submodule_repo_env(struct argv_array *out);
+/*
+ * Embed a git dir of the submodule given by path.
+ */
+#define RELOCATE_GITDIR_RECURSE_SUBMODULES (1<<0)
+extern int submodule_embed_git_dir(const char *prefix,
+ const char *path,
+ unsigned flags);
#endif
--
2.11.0.rc2.28.g2af45f1.dirty
^ permalink raw reply related
* Re: [PATCH 1/3] wt-status: implement opportunisitc index update correctly
From: Stefan Beller @ 2016-12-07 20:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Paul Tan
In-Reply-To: <20161207194105.25780-2-gitster@pobox.com>
On Wed, Dec 7, 2016 at 11:41 AM, Junio C Hamano <gitster@pobox.com> wrote:
> The require_clean_work_tree() function calls hold_locked_index()
> with die_on_error=0 to signal that it is OK if it fails to obtain
> the lock, but unconditionally calls update_index_if_able(), which
> will try to write into fd=-1.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
So my first question I had to answer was if we do the right thing here,
i.e. if we could just fail instead. But we want to continue and just
not write back the index, which is fine.
So we do not have to guard refresh_cache, but just call
update_index_if_able conditionally.
However I think the promise of that function is
to take care of the fd == -1?
/*
* Opportunistically update the index but do not complain if we can't
*/
void update_index_if_able(struct index_state *istate, struct
lock_file *lockfile)
{
if ((istate->cache_changed || has_racy_timestamp(istate)) &&
verify_index(istate) &&
write_locked_index(istate, lockfile, COMMIT_LOCK))
rollback_lock_file(lockfile);
}
So I would expect that we'd rather fix the update_index_if_able instead by
checking for the lockfile to be in the correct state?
> wt-status.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index a2e9d332d8..a715e71906 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -2258,11 +2258,12 @@ int has_uncommitted_changes(int ignore_submodules)
> int require_clean_work_tree(const char *action, const char *hint, int ignore_submodules, int gently)
> {
> struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
> - int err = 0;
> + int err = 0, fd;
>
> - hold_locked_index(lock_file, 0);
> + fd = hold_locked_index(lock_file, 0);
> refresh_cache(REFRESH_QUIET);
> - update_index_if_able(&the_index, lock_file);
> + if (0 <= fd)
> + update_index_if_able(&the_index, lock_file);
> rollback_lock_file(lock_file);
>
> if (has_unstaged_changes(ignore_submodules)) {
> --
> 2.11.0-274-g0631465056
>
^ permalink raw reply
* Re: [PATCH 1/3] wt-status: implement opportunisitc index update correctly
From: Stefan Beller @ 2016-12-07 20:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Paul Tan
In-Reply-To: <CAPc5daX2CZ0=UBMuz70KwFPBDTUgAdi8WoVUJ7gNTq+QEXKxbg@mail.gmail.com>
On Wed, Dec 7, 2016 at 12:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> On Wed, Dec 7, 2016 at 12:48 PM, Stefan Beller <sbeller@google.com> wrote:
>>
>> So I would expect that we'd rather fix the update_index_if_able instead by
>> checking for the lockfile to be in the correct state?
>
> I actually don't expect that, after looking at other call sites of
> that function.
Yes I checked the other callers as well right now and you seem to be correct.
My initial response was based on the name of the function,
specifically the _if_able
part as that hinted to me that I can call the function with no
precondition and the
_if_able will figure out when to do the actual update_index.
The first part of the condition of the function
(istate->cache_changed || has_racy_timestamp(istate)
reads rather as a _if_needed instead of an _if_able to me.
^ permalink raw reply
* Re: [PATCH 1/3] wt-status: implement opportunisitc index update correctly
From: Junio C Hamano @ 2016-12-07 20:53 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Paul Tan
In-Reply-To: <CAGZ79kZHGqU2y19_uKhtVuE6vhspzPNpw-nVDnm8gLQ8u528kQ@mail.gmail.com>
On Wed, Dec 7, 2016 at 12:48 PM, Stefan Beller <sbeller@google.com> wrote:
>
> So I would expect that we'd rather fix the update_index_if_able instead by
> checking for the lockfile to be in the correct state?
I actually don't expect that, after looking at other call sites of
that function.
^ permalink raw reply
* Re: [PATCH] real_path: make real_path thread-safe
From: Johannes Sixt @ 2016-12-07 20:43 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, git, sbeller, peff, jacob.keller
In-Reply-To: <20161207001018.GD103573@google.com>
Am 07.12.2016 um 01:10 schrieb Brandon Williams:
> This function should accept both absolute and relative paths, which
> means it should probably accept "C:\My Files". I wasn't thinking about
> windows 100% of the time while writing this so I'm hoping that a windows
> expert will point things like this out to me :).
;)
With this patch, the test suite fails at the very first git init call:
D:\Src\mingw-git\t>sh t0000-basic.sh -v -i -x
fatal: Invalid path '/:': No such file or directory
error: cannot run git init -- have you built things yet?
FATAL: Unexpected exit with code 1
I haven't dug further, yet.
-- Hannes
^ permalink raw reply
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