>>> The final version needs to be accompanied with tests to show the
>>> effect of this change for callers. A test would set up a top-level
>>> and submodule, deliberately break submodule/.git/ repository and
>>> show what breaks and how without this change.
Agreed!
The repo where the original p
> The final version needs to be accompanied with tests to show the
> effect of this change for callers. A test would set up a top-level
> and submodule, deliberately break submodule/.git/ repository and
> show what breaks and how without this change.
Tests are really good at providing this contex
On Thu, Sep 1, 2016 at 1:21 PM, Junio C Hamano wrote:
> Junio C Hamano writes:
>
>> Stefan Beller writes:
>>
The final version needs to be accompanied with tests to show the
effect of this change for callers. A test would set up a top-level
and submodule, deliberately break submo
Uma Srinivasan writes:
> Yes, this one line fix addresses my problem.
>
> So, what is the next step? Will someone submit a patch or should I?
> Please note that I've never submitted a patch before, but I don't mind
> learning how to.
The final version needs to be accompanied with tests to show t
Stefan Beller writes:
>> +test_expect_success 'fetching submodule into a broken repository' '
>> + # Prepare src and src/sub nested in it
>> + git init src &&
>> + (
>> + cd src &&
>> + git init sub &&
>> + git -C sub commit --allow-empt
Junio C Hamano writes:
If we add
# "git diff" should terminate with an error.
# NOTE: without fix this will recurse forever!
test_must_fail git -C dst diff &&
after breaking the repository, we can also see "git diff" recurse
forever, because it wants to know if "sub" sub
Uma Srinivasan writes:
> Anyway, with the fix "git status" fails quickly both in my reproducer
> (and original repo) with the following message.
> fatal: Not a git repository: '.git'
> fatal: 'git status --porcelain' failed in submodule commonlib
Thanks, that is exactly what I wanted to see
Junio C Hamano writes:
> Stefan Beller writes:
>
>>> The final version needs to be accompanied with tests to show the
>>> effect of this change for callers. A test would set up a top-level
>>> and submodule, deliberately break submodule/.git/ repository and
>>> show what breaks and how without
Stefan Beller writes:
>> The final version needs to be accompanied with tests to show the
>> effect of this change for callers. A test would set up a top-level
>> and submodule, deliberately break submodule/.git/ repository and
>> show what breaks and how without this change.
>
> Tests are reall
Yes, this one line fix addresses my problem.
So, what is the next step? Will someone submit a patch or should I?
Please note that I've never submitted a patch before, but I don't mind
learning how to.
Thanks,
Uma
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1160,4 +1160,5 @@ void prepare_submod
Junio C Hamano writes:
> I was wondering if we should unconditionally stuff GIT_DIR= repository location for the submodule> in the cp.env_array passed to
> the function prepare_submodule_repo_env(). As cp.dir will be set to
> the submodule's working tree, there is no need to set GIT_WORK_TREE
>
Here's one more attempt where I changed prepare_submodule_repo_env()
to take the submodule git dir as an argument.
Sorry for including this long code diff inline. If there's a better
way to have this discussion with code please let me know.
Thanks,
Uma
diff --git a/builtin/submodule--helper.c b/b
> We want to affect only the process we are going to spawn to work
> inside the submodule, not ourselves, which is what this call does;
> this does not sound like a good idea.
Okay, in that case I would have to pass the "git_dir" as a new
argument to prepare_submodule_repo_env(). I know what to pa
Uma Srinivasan writes:
> diff --git a/submodule.c b/submodule.c
> index 5a62aa2..23443a7 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -960,6 +960,9 @@ unsigned is_submodule_modified(const char *path,
> int ignore_untracked)
> return 0;
>
> }
> + /* stuff submo
FWIW, I tried the following quick change based on the info. provided
by Junio and it seems to "fix" my original problem. I'll let you
experts figure out if we need a more complete fix or not.
diff --git a/submodule.c b/submodule.c
index 5a62aa2..23443a7 100644
--- a/submodule.c
+++ b/submodule.c
@
Uma Srinivasan writes:
>> I might suggest to update prepare_submodule_repo_env() so that the
>> spawned process will *NOT* have to guess where the working tree and
>> repository by exporting GIT_DIR (set to "git_dir" we discover above)
>> and GIT_WORK_TREE (set to "." as cp.dir is set to the path
On Tue, Aug 30, 2016 at 10:53 AM, Junio C Hamano wrote:
> Uma Srinivasan writes:
>
>> I think the following fix is still needed to is_submodule_modified():
>>
>> strbuf_addf(&buf, "%s/.git", path);
>> git_dir = read_gitfile(buf.buf);
>> if (!git_dir) {
>> g
Uma Srinivasan writes:
> I think the following fix is still needed to is_submodule_modified():
>
> strbuf_addf(&buf, "%s/.git", path);
> git_dir = read_gitfile(buf.buf);
> if (!git_dir) {
> git_dir = buf.buf;
> ==> if (!is_git_directory(git_d
Thanks for the patch. Unfortunately, it doesn't help in my case as it
invokes the is_submodule_modified() routine which you didn't modify.
Here's my call trace
#0 is_submodule_modified (path=path@entry=0x17c2f08 "groc", ignore_untracked=0)
at submodule.c:939
#1 0x004aa4dc in matc
On Mon, Aug 29, 2016 at 11:09 PM, Jacob Keller wrote:
> On Mon, Aug 29, 2016 at 5:12 PM, Uma Srinivasan
> wrote:
>> This is great! Thanks Jake. If you happen to have the patch ID it
>> would be helpful.
>>
>> Uma
>>
>
> http://public-inbox.org/git/1472236108.28343.5.ca...@intel.com/
Actually c
On Mon, Aug 29, 2016 at 5:12 PM, Uma Srinivasan wrote:
> This is great! Thanks Jake. If you happen to have the patch ID it
> would be helpful.
>
> Uma
>
http://public-inbox.org/git/1472236108.28343.5.ca...@intel.com/
This is great! Thanks Jake. If you happen to have the patch ID it
would be helpful.
Uma
On Mon, Aug 29, 2016 at 5:02 PM, Jacob Keller wrote:
> On Mon, Aug 29, 2016 at 4:15 PM, Junio C Hamano wrote:
>> Uma Srinivasan writes:
>>> This fixes my issue but what do you think? Is this the right way t
On Mon, Aug 29, 2016 at 4:15 PM, Junio C Hamano wrote:
> Uma Srinivasan writes:
>> This fixes my issue but what do you think? Is this the right way to
>> fix it? Is there a better way?
>
> I think we already have a helper function that does a lot better
> than "does it have a file called HEAD" to
Yes, is_git_directory() is much better. Thanks for the pointer.
I will submit a patch unless I hear more suggestions from others.
Uma
On Mon, Aug 29, 2016 at 4:15 PM, Junio C Hamano wrote:
> Uma Srinivasan writes:
>
>> On Mon, Aug 29, 2016 at 2:13 PM, Uma Srinivasan
>> wrote:
>>> Ok that m
Uma Srinivasan writes:
> On Mon, Aug 29, 2016 at 2:13 PM, Uma Srinivasan
> wrote:
>> Ok that makes sense. Thanks much.
>>
>> Uma
>>
> With respect to my original problem with a corrupted .git directory
> under the submodule directory, I am thinking of adding the following 4
> lines marked with
With respect to my original problem with a corrupted .git directory
under the submodule directory, I am thinking of adding the following 4
lines marked with ### to is_submodule_modified() to detect the
corrupted dir and die quickly instead of forking several child
processes:
strbuf_
Ok that makes sense. Thanks much.
Uma
On Mon, Aug 29, 2016 at 2:09 PM, Junio C Hamano wrote:
> On Mon, Aug 29, 2016 at 2:03 PM, Uma Srinivasan
> wrote:
>> On Mon, Aug 29, 2016 at 1:03 PM, Junio C Hamano wrote:
>>>
>>> A top-level superproject can have a submodule bound at its "dir/"
>>> direc
Thanks for the reply. However, in this case
git clone $URL ./dir2
git add dir2
how will "dir2" get ever get registered as a submodule? I don't see
how one can reach the "is_submodule_modified" routine for the scenario
above.
My understanding is that a sub-directory can be registe
On Mon, Aug 29, 2016 at 2:03 PM, Uma Srinivasan wrote:
> On Mon, Aug 29, 2016 at 1:03 PM, Junio C Hamano wrote:
>>
>> A top-level superproject can have a submodule bound at its "dir/"
>> directory, and "dir/.git" can either be a gitfile which you can read
>> with read_gitfile() and point into som
Uma Srinivasan writes:
> git_dir = read_gitfile(buf.buf);
> if (!git_dir)
>
> git_dir = buf.buf;
>
> Can anyone explain to me why we are replacing a failed reading of a
> git file with the original sub directory name?
A top-level superproject can have a submodule bound at its "di
Hi,
I am new to git source and internal development. Recently I've been
looking at a problem where issuing "git status" in a corrupted
workspace handed over to me by a user, forks several thousand child
processes recursively.
The symptoms of the corrupted workspace to reproduce this problem are
a
31 matches
Mail list logo
