Re: git submodules implementation question

>>> 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

Re: git submodules implementation question

> 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

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

> 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

Re: git submodules implementation question

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; > >

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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(, "%s/.git", path); >> git_dir = read_gitfile(buf.buf); >>

Re: git submodules implementation question

Uma Srinivasan writes: > I think the following fix is still needed to is_submodule_modified(): > > strbuf_addf(, "%s/.git", path); > git_dir = read_gitfile(buf.buf); > if (!git_dir) { > git_dir = buf.buf; > ==> if

Re: git submodules implementation question

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 >> > >

Re: git submodules implementation question

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/

Re: git submodules implementation question

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:

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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:

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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

Re: git submodules implementation question

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