* [PATCH] commit: configure submodules @ 2012-09-23 7:37 Orgad Shaneh 2012-09-23 8:37 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Orgad Shaneh @ 2012-09-23 7:37 UTC (permalink / raw) To: git; +Cc: Orgad Shaneh As reported on the mailing list[1], ignore submodule config is not respected on commit. [1] http://git.661346.n2.nabble.com/submodule-ignore-is-not-respected-on-commit-td7539238.html Signed-off-by: Orgad Shaneh <orgads@gmail.com> --- builtin/commit.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/commit.c b/builtin/commit.c index 62028e7..7a83cae 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1452,6 +1452,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) usage_with_options(builtin_commit_usage, builtin_commit_options); wt_status_prepare(&s); + gitmodules_config(); git_config(git_commit_config, &s); determine_whence(&s); s.colopts = 0; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-23 7:37 [PATCH] commit: configure submodules Orgad Shaneh @ 2012-09-23 8:37 ` Junio C Hamano 2012-09-23 18:13 ` Jens Lehmann 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2012-09-23 8:37 UTC (permalink / raw) To: Orgad Shaneh; +Cc: git, Jens Lehmann Orgad Shaneh <orgads@gmail.com> writes: > As reported on the mailing list[1], ignore submodule config is not > respected on commit. > > [1] http://git.661346.n2.nabble.com/submodule-ignore-is-not-respected-on-commit-td7539238.html > > Signed-off-by: Orgad Shaneh <orgads@gmail.com> > --- Thanks. Please do not force people to go to external website like that while reading the "git log" output. You could have just said "git status" does not list a submodule with uncommitted working tree files as modified when "submodule.$name.ignore" configuration is set to "dirty", but "git commit" forgets to take this configuration variable into account. or something. It would also be good to say "I reported it earlier in this message" with the URL _after_ the three-dash line. I see Jens added with 302ad7a (Submodules: Use "ignore" settings from .gitmodules too for diff and status, 2010-08-06) the call to gitmodules_config() to "git status" and "git diff" family, but I suspect that was a huge mistake. Once a submodule is initialized with "submodule init", the default set of configuration should be copied to the user's $GIT_DIR/config and subsequent run-time invocation should read $GIT_DIR/config and $GIT_DIR/config alone, to honor user's customization. Instead, I think git_commit_config() and git_status_config() should call submodule_config() function to read submodule.$name.ignore not from .gitmodules file but from $GIT_DIR/config. Jens, what do you think? I see no reason for anybody other than "submodule init" to call gitmodules_config() that reads from the in-tree .gitmodules file. > builtin/commit.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/builtin/commit.c b/builtin/commit.c > index 62028e7..7a83cae 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -1452,6 +1452,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) > usage_with_options(builtin_commit_usage, builtin_commit_options); > > wt_status_prepare(&s); > + gitmodules_config(); > git_config(git_commit_config, &s); > determine_whence(&s); > s.colopts = 0; ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-23 8:37 ` Junio C Hamano @ 2012-09-23 18:13 ` Jens Lehmann 2012-09-24 5:57 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Jens Lehmann @ 2012-09-23 18:13 UTC (permalink / raw) To: Junio C Hamano; +Cc: Orgad Shaneh, git Am 23.09.2012 10:37, schrieb Junio C Hamano: > I see Jens added with 302ad7a (Submodules: Use "ignore" settings > from .gitmodules too for diff and status, 2010-08-06) the call to > gitmodules_config() to "git status" and "git diff" family, but I > suspect that was a huge mistake. Once a submodule is initialized > with "submodule init", the default set of configuration should be > copied to the user's $GIT_DIR/config and subsequent run-time > invocation should read $GIT_DIR/config and $GIT_DIR/config alone, to > honor user's customization. Not honoring the user's customization would be a big mistake, but this is not what happens here. A setting in $GIT_DIR/config always overrides the one in .gitmodules (that's why gitmodules_config() is called before git_config()). > Instead, I think git_commit_config() and git_status_config() should > call submodule_config() function to read submodule.$name.ignore not > from .gitmodules file but from $GIT_DIR/config. > > Jens, what do you think? I see no reason for anybody other than > "submodule init" to call gitmodules_config() that reads from the > in-tree .gitmodules file. I think the copying on init is not what we should do here because it sets the user's customization to what ever happened to be in .gitmodules at the time he initialized the submodule. Later changes from upstream to such a setting would not be honored unless the user copies that new setting herself (which I think is The Right Thing for the URL, but not for the other work tree related settings like 'ignore'). Imagine you have a submodule containing a huge media file which is set to be ignored for performance reasons. When upstream later decides it should rather use .gitattributes to just disable diffing that file and removes the submodule ignore so the users see changes to other files of the submodule again, that will just work the way it is done now, but won't when we copy that setting on init. So it is either "honor upstream unless the user decides otherwise" or "take what upstream configured at init time as the users choice (until he actively changes it)". And I think the former is more flexible as it allows upstream to change settings without user intervention, which is why I did it that way. And as I understand that .gitattributes follow the same principle: Unless the user configured something different in his $GIT_DIR/info/attributes file, git will use the settings from the .gitattributes file of the currently checked out commit. So I think Orgad's change is sane and should go in. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-23 18:13 ` Jens Lehmann @ 2012-09-24 5:57 ` Junio C Hamano 2012-09-24 16:27 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2012-09-24 5:57 UTC (permalink / raw) To: Jens Lehmann; +Cc: Orgad Shaneh, git Jens Lehmann <Jens.Lehmann@web.de> writes: >> Jens, what do you think? I see no reason for anybody other than >> "submodule init" to call gitmodules_config() that reads from the >> in-tree .gitmodules file. > > I think the copying on init is not what we should do here because > it sets the user's customization to what ever happened to be in > .gitmodules at the time he initialized the submodule. Hrm, why does the user have submodule.$name.$whatever customized before saying "submodule init $whategver" for that copying to be problematic? > So I think Orgad's change is sane and should go in. Matching what cmd_commit() does to what cmd_status() does, i.e. grab submodule.$name.ignore from somewhere, is not something I questioned. The patch is a good change to make them consistent. What I was wondering was if that is a consistently wrong thing to do to read from .gitmodules not $GIT_DIR/config. In any case, the log message I suggested in the review needs to be updated in the reroll to make it clear that this is about reading from .gitmodules, not "configuration". AFAICS, gitmodule_config() does not even read from $GIT_DIR/config, right? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-24 5:57 ` Junio C Hamano @ 2012-09-24 16:27 ` Junio C Hamano 2012-09-24 18:15 ` Jens Lehmann 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2012-09-24 16:27 UTC (permalink / raw) To: Jens Lehmann; +Cc: Orgad Shaneh, git Junio C Hamano <gitster@pobox.com> writes: > Jens Lehmann <Jens.Lehmann@web.de> writes: > >>> Jens, what do you think? I see no reason for anybody other than >>> "submodule init" to call gitmodules_config() that reads from the >>> in-tree .gitmodules file. >> >> I think the copying on init is not what we should do here because >> it sets the user's customization to what ever happened to be in >> .gitmodules at the time he initialized the submodule. > > Hrm, why does the user have submodule.$name.$whatever customized > before saying "submodule init $whategver" for that copying to be > problematic? > >> So I think Orgad's change is sane and should go in. > > Matching what cmd_commit() does to what cmd_status() does, i.e. grab > submodule.$name.ignore from somewhere, is not something I questioned. > The patch is a good change to make them consistent. > > What I was wondering was if that is a consistently wrong thing to do > to read from .gitmodules not $GIT_DIR/config. > > In any case, the log message I suggested in the review needs to be > updated in the reroll to make it clear that this is about reading > from .gitmodules, not "configuration". AFAICS, gitmodule_config() > does not even read from $GIT_DIR/config, right? OK. gitmodule_config() does not read $GIT_DIR/config, but cmd_status() and cmd_commit() call git_diff_basic_config() that is called from git_diff_ui_config() to read submodule.$name.ignore from it. So Orgad's patch is _only_ about submodule.$name.ignore that is in in-tree .gitmodule; the log message shouldn't mention "config", as setting configuration variables work for both status and commit just fine. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-24 16:27 ` Junio C Hamano @ 2012-09-24 18:15 ` Jens Lehmann 2012-09-24 18:34 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Jens Lehmann @ 2012-09-24 18:15 UTC (permalink / raw) To: Junio C Hamano; +Cc: Orgad Shaneh, git Am 24.09.2012 18:27, schrieb Junio C Hamano: > Junio C Hamano <gitster@pobox.com> writes: >> In any case, the log message I suggested in the review needs to be >> updated in the reroll to make it clear that this is about reading >> from .gitmodules, not "configuration". AFAICS, gitmodule_config() >> does not even read from $GIT_DIR/config, right? > > OK. gitmodule_config() does not read $GIT_DIR/config, but > cmd_status() and cmd_commit() call git_diff_basic_config() that is > called from git_diff_ui_config() to read submodule.$name.ignore from > it. So Orgad's patch is _only_ about submodule.$name.ignore that is > in in-tree .gitmodule; the log message shouldn't mention "config", > as setting configuration variables work for both status and commit > just fine. Yes, I was just checking that call path too to make sure the user settings from $GIT_DIR/config really override those found in .gitmodules. And of course you are right, while the change to the code is sane the commit message still needs some work. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-24 18:15 ` Jens Lehmann @ 2012-09-24 18:34 ` Junio C Hamano 2012-09-24 18:49 ` Orgad Shaneh 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2012-09-24 18:34 UTC (permalink / raw) To: Jens Lehmann; +Cc: Orgad Shaneh, git Jens Lehmann <Jens.Lehmann@web.de> writes: > Am 24.09.2012 18:27, schrieb Junio C Hamano: >> Junio C Hamano <gitster@pobox.com> writes: >>> In any case, the log message I suggested in the review needs to be >>> updated in the reroll to make it clear that this is about reading >>> from .gitmodules, not "configuration". AFAICS, gitmodule_config() >>> does not even read from $GIT_DIR/config, right? >> >> OK. gitmodule_config() does not read $GIT_DIR/config, but >> cmd_status() and cmd_commit() call git_diff_basic_config() that is >> called from git_diff_ui_config() to read submodule.$name.ignore from >> it. So Orgad's patch is _only_ about submodule.$name.ignore that is >> in in-tree .gitmodule; the log message shouldn't mention "config", >> as setting configuration variables work for both status and commit >> just fine. > > Yes, I was just checking that call path too to make sure the user > settings from $GIT_DIR/config really override those found in > .gitmodules. And of course you are right, while the change to the > code is sane the commit message still needs some work. Here is what I tentatively queued on 'pu' (not pushed out yet). -- >8 -- From: Orgad Shaneh <orgads@gmail.com> Date: Sun, 23 Sep 2012 09:37:47 +0200 Subject: [PATCH] commit: pay attention to submodule.$name.ignore in .gitmodules "git status" does not list a submodule with uncommitted working tree files as modified when "submodule.$name.ignore" is set to "dirty" in in-tree ".gitmodules" file. Both status and commit honor the setting in $GIT_DIR/config, but "commit" does not pick it up from .gitmodules, which is inconsistent. Teach "git commit" to pay attention to the setting in .gitmodules as well. Signed-off-by: Orgad Shaneh <orgads@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/commit.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/commit.c b/builtin/commit.c index 66fdd22..3cb1ef7 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1256,6 +1256,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) struct wt_status s; wt_status_prepare(&s); + gitmodules_config(); git_config(git_commit_config, &s); in_merge = file_exists(git_path("MERGE_HEAD")); s.in_merge = in_merge; -- 1.7.12.1.441.g794a63b ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-24 18:34 ` Junio C Hamano @ 2012-09-24 18:49 ` Orgad Shaneh 2012-09-24 19:06 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Orgad Shaneh @ 2012-09-24 18:49 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jens Lehmann, git On Mon, Sep 24, 2012 at 8:34 PM, Junio C Hamano <gitster@pobox.com> wrote: > > Jens Lehmann <Jens.Lehmann@web.de> writes: > > > Am 24.09.2012 18:27, schrieb Junio C Hamano: > >> Junio C Hamano <gitster@pobox.com> writes: > >>> In any case, the log message I suggested in the review needs to be > >>> updated in the reroll to make it clear that this is about reading > >>> from .gitmodules, not "configuration". AFAICS, gitmodule_config() > >>> does not even read from $GIT_DIR/config, right? > >> > >> OK. gitmodule_config() does not read $GIT_DIR/config, but > >> cmd_status() and cmd_commit() call git_diff_basic_config() that is > >> called from git_diff_ui_config() to read submodule.$name.ignore from > >> it. So Orgad's patch is _only_ about submodule.$name.ignore that is > >> in in-tree .gitmodule; the log message shouldn't mention "config", > >> as setting configuration variables work for both status and commit > >> just fine. > > > > Yes, I was just checking that call path too to make sure the user > > settings from $GIT_DIR/config really override those found in > > .gitmodules. And of course you are right, while the change to the > > code is sane the commit message still needs some work. > > Here is what I tentatively queued on 'pu' (not pushed out yet). > > -- >8 -- > From: Orgad Shaneh <orgads@gmail.com> > Date: Sun, 23 Sep 2012 09:37:47 +0200 > Subject: [PATCH] commit: pay attention to submodule.$name.ignore in > .gitmodules > > "git status" does not list a submodule with uncommitted working tree > files as modified when "submodule.$name.ignore" is set to "dirty" in > in-tree ".gitmodules" file. Both status and commit honor the setting > in $GIT_DIR/config, but "commit" does not pick it up from .gitmodules, > which is inconsistent. > > Teach "git commit" to pay attention to the setting in .gitmodules as > well. > > Signed-off-by: Orgad Shaneh <orgads@gmail.com> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > builtin/commit.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/builtin/commit.c b/builtin/commit.c > index 66fdd22..3cb1ef7 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -1256,6 +1256,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) > struct wt_status s; > > wt_status_prepare(&s); > + gitmodules_config(); > git_config(git_commit_config, &s); > in_merge = file_exists(git_path("MERGE_HEAD")); > s.in_merge = in_merge; > -- > 1.7.12.1.441.g794a63b > That is not correct. git-config is ignored as well for commit. Maybe another fix is needed? - Orgad ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-24 18:49 ` Orgad Shaneh @ 2012-09-24 19:06 ` Junio C Hamano 2012-09-24 19:16 ` Orgad Shaneh 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2012-09-24 19:06 UTC (permalink / raw) To: Orgad Shaneh; +Cc: Jens Lehmann, git Orgad Shaneh <orgads@gmail.com> writes: > That is not correct. git-config is ignored as well for commit. What do you mean? As far as I can tell, if you have [submodule "var"] path = var ignore = dirty in $GIT_DIR/config, a work-tree-dirty submodule "var" is not reported by "git status" and "git commit" without your patch, and your patch does not seem to break that. The only difference your patch makes is that if you had the above three-line block in the .gitmodules file and not in $GIT_DIR/config, "git status" ignored the dirtyness in the working tree, but "git commit" did notice and report it. What am I missing? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-24 19:06 ` Junio C Hamano @ 2012-09-24 19:16 ` Orgad Shaneh 2012-09-24 19:56 ` Jens Lehmann 0 siblings, 1 reply; 14+ messages in thread From: Orgad Shaneh @ 2012-09-24 19:16 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jens Lehmann, git On Mon, Sep 24, 2012 at 9:06 PM, Junio C Hamano <gitster@pobox.com> wrote: > Orgad Shaneh <orgads@gmail.com> writes: > >> That is not correct. git-config is ignored as well for commit. > > What do you mean? As far as I can tell, if you have > > [submodule "var"] > path = var > ignore = dirty > > in $GIT_DIR/config, a work-tree-dirty submodule "var" is not > reported by "git status" and "git commit" without your patch, and > your patch does not seem to break that. The only difference your > patch makes is that if you had the above three-line block in > the .gitmodules file and not in $GIT_DIR/config, "git status" > ignored the dirtyness in the working tree, but "git commit" did > notice and report it. > > What am I missing? > > I have: [submodule "mod"] url = [...] ignore = dirty in .git/config, and I removed the ignore part from .gitmodules to be even. I made a change inside mod, git status doesn't report its dirtiness, while git commit does. git status: # On branch master # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # modified: foo # # Changes not staged for commit: # (use "git add <file>..." to update what will be committed) # (use "git checkout -- <file>..." to discard changes in working directory) # # modified: .gitmodules # git commit: # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # On branch master # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # modified: foo # # Changes not staged for commit: # (use "git add <file>..." to update what will be committed) # (use "git checkout -- <file>..." to discard changes in working directory) # (commit or discard the untracked or modified content in submodules) # # modified: .gitmodules # modified: mod (modified content) # Now I get it! That's because I don't have submodule.mod.path! config_name_for_path only gets initialized if path exists. Apparently git submodule init doesn't configure 'path', so it stays uninitialized. - Orgad ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-24 19:16 ` Orgad Shaneh @ 2012-09-24 19:56 ` Jens Lehmann 2012-09-24 19:59 ` Orgad Shaneh 2012-09-24 20:36 ` Junio C Hamano 0 siblings, 2 replies; 14+ messages in thread From: Jens Lehmann @ 2012-09-24 19:56 UTC (permalink / raw) To: Orgad Shaneh; +Cc: Junio C Hamano, git Am 24.09.2012 21:16, schrieb Orgad Shaneh: > On Mon, Sep 24, 2012 at 9:06 PM, Junio C Hamano <gitster@pobox.com> wrote: >> Orgad Shaneh <orgads@gmail.com> writes: >> >>> That is not correct. git-config is ignored as well for commit. >> >> What do you mean? As far as I can tell, if you have >> >> [submodule "var"] >> path = var >> ignore = dirty >> >> in $GIT_DIR/config, a work-tree-dirty submodule "var" is not >> reported by "git status" and "git commit" without your patch, and >> your patch does not seem to break that. The only difference your >> patch makes is that if you had the above three-line block in >> the .gitmodules file and not in $GIT_DIR/config, "git status" >> ignored the dirtyness in the working tree, but "git commit" did >> notice and report it. >> >> What am I missing? >> >> > > I have: > [submodule "mod"] > url = [...] > ignore = dirty > > in .git/config, and I removed the ignore part from .gitmodules to be even. > > I made a change inside mod, git status doesn't report its dirtiness, > while git commit does. > > git status: > # On branch master > # Changes to be committed: > # (use "git reset HEAD <file>..." to unstage) > # > # modified: foo > # > # Changes not staged for commit: > # (use "git add <file>..." to update what will be committed) > # (use "git checkout -- <file>..." to discard changes in working directory) > # > # modified: .gitmodules > # > > git commit: > # Please enter the commit message for your changes. Lines starting > # with '#' will be ignored, and an empty message aborts the commit. > # On branch master > # Changes to be committed: > # (use "git reset HEAD <file>..." to unstage) > # > # modified: foo > # > # Changes not staged for commit: > # (use "git add <file>..." to update what will be committed) > # (use "git checkout -- <file>..." to discard changes in working directory) > # (commit or discard the untracked or modified content in submodules) > # > # modified: .gitmodules > # modified: mod (modified content) > # > > Now I get it! That's because I don't have submodule.mod.path! > config_name_for_path only gets initialized if path exists. Apparently > git submodule init doesn't configure 'path', so it stays > uninitialized. But submodule.mod.path should only be set in .gitmodules, not in $GIT_DIR/config. Did you just remove the ignore setting from .gitmodules or the path too? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-24 19:56 ` Jens Lehmann @ 2012-09-24 19:59 ` Orgad Shaneh 2012-09-24 20:10 ` Jens Lehmann 2012-09-24 20:36 ` Junio C Hamano 1 sibling, 1 reply; 14+ messages in thread From: Orgad Shaneh @ 2012-09-24 19:59 UTC (permalink / raw) To: Jens Lehmann; +Cc: Junio C Hamano, git On Mon, Sep 24, 2012 at 9:56 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote: > Am 24.09.2012 21:16, schrieb Orgad Shaneh: >> On Mon, Sep 24, 2012 at 9:06 PM, Junio C Hamano <gitster@pobox.com> wrote: >>> Orgad Shaneh <orgads@gmail.com> writes: >>> >>>> That is not correct. git-config is ignored as well for commit. >>> >>> What do you mean? As far as I can tell, if you have >>> >>> [submodule "var"] >>> path = var >>> ignore = dirty >>> >>> in $GIT_DIR/config, a work-tree-dirty submodule "var" is not >>> reported by "git status" and "git commit" without your patch, and >>> your patch does not seem to break that. The only difference your >>> patch makes is that if you had the above three-line block in >>> the .gitmodules file and not in $GIT_DIR/config, "git status" >>> ignored the dirtyness in the working tree, but "git commit" did >>> notice and report it. >>> >>> What am I missing? >>> >>> >> >> I have: >> [submodule "mod"] >> url = [...] >> ignore = dirty >> >> in .git/config, and I removed the ignore part from .gitmodules to be even. >> >> I made a change inside mod, git status doesn't report its dirtiness, >> while git commit does. >> >> git status: >> # On branch master >> # Changes to be committed: >> # (use "git reset HEAD <file>..." to unstage) >> # >> # modified: foo >> # >> # Changes not staged for commit: >> # (use "git add <file>..." to update what will be committed) >> # (use "git checkout -- <file>..." to discard changes in working directory) >> # >> # modified: .gitmodules >> # >> >> git commit: >> # Please enter the commit message for your changes. Lines starting >> # with '#' will be ignored, and an empty message aborts the commit. >> # On branch master >> # Changes to be committed: >> # (use "git reset HEAD <file>..." to unstage) >> # >> # modified: foo >> # >> # Changes not staged for commit: >> # (use "git add <file>..." to update what will be committed) >> # (use "git checkout -- <file>..." to discard changes in working directory) >> # (commit or discard the untracked or modified content in submodules) >> # >> # modified: .gitmodules >> # modified: mod (modified content) >> # >> >> Now I get it! That's because I don't have submodule.mod.path! >> config_name_for_path only gets initialized if path exists. Apparently >> git submodule init doesn't configure 'path', so it stays >> uninitialized. > > But submodule.mod.path should only be set in .gitmodules, not in > $GIT_DIR/config. Did you just remove the ignore setting from > .gitmodules or the path too? Just the ignore, and my patch of course. If it is not set in $GIT_DIR/config, then config_name_for_path is not initialized, and if it is not initialized, then set_diffopt_flags_from_submodule_config does nothing (handle_ignore_submodules_arg is not called). That is the main problem. - Orgad ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-24 19:59 ` Orgad Shaneh @ 2012-09-24 20:10 ` Jens Lehmann 0 siblings, 0 replies; 14+ messages in thread From: Jens Lehmann @ 2012-09-24 20:10 UTC (permalink / raw) To: Orgad Shaneh; +Cc: Junio C Hamano, git Am 24.09.2012 21:59, schrieb Orgad Shaneh: > On Mon, Sep 24, 2012 at 9:56 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote: >> Am 24.09.2012 21:16, schrieb Orgad Shaneh: >>> On Mon, Sep 24, 2012 at 9:06 PM, Junio C Hamano <gitster@pobox.com> wrote: >>>> Orgad Shaneh <orgads@gmail.com> writes: >>>> >>>>> That is not correct. git-config is ignored as well for commit. >>>> >>>> What do you mean? As far as I can tell, if you have >>>> >>>> [submodule "var"] >>>> path = var >>>> ignore = dirty >>>> >>>> in $GIT_DIR/config, a work-tree-dirty submodule "var" is not >>>> reported by "git status" and "git commit" without your patch, and >>>> your patch does not seem to break that. The only difference your >>>> patch makes is that if you had the above three-line block in >>>> the .gitmodules file and not in $GIT_DIR/config, "git status" >>>> ignored the dirtyness in the working tree, but "git commit" did >>>> notice and report it. >>>> >>>> What am I missing? >>>> >>>> >>> >>> I have: >>> [submodule "mod"] >>> url = [...] >>> ignore = dirty >>> >>> in .git/config, and I removed the ignore part from .gitmodules to be even. >>> >>> I made a change inside mod, git status doesn't report its dirtiness, >>> while git commit does. >>> >>> git status: >>> # On branch master >>> # Changes to be committed: >>> # (use "git reset HEAD <file>..." to unstage) >>> # >>> # modified: foo >>> # >>> # Changes not staged for commit: >>> # (use "git add <file>..." to update what will be committed) >>> # (use "git checkout -- <file>..." to discard changes in working directory) >>> # >>> # modified: .gitmodules >>> # >>> >>> git commit: >>> # Please enter the commit message for your changes. Lines starting >>> # with '#' will be ignored, and an empty message aborts the commit. >>> # On branch master >>> # Changes to be committed: >>> # (use "git reset HEAD <file>..." to unstage) >>> # >>> # modified: foo >>> # >>> # Changes not staged for commit: >>> # (use "git add <file>..." to update what will be committed) >>> # (use "git checkout -- <file>..." to discard changes in working directory) >>> # (commit or discard the untracked or modified content in submodules) >>> # >>> # modified: .gitmodules >>> # modified: mod (modified content) >>> # >>> >>> Now I get it! That's because I don't have submodule.mod.path! >>> config_name_for_path only gets initialized if path exists. Apparently >>> git submodule init doesn't configure 'path', so it stays >>> uninitialized. >> >> But submodule.mod.path should only be set in .gitmodules, not in >> $GIT_DIR/config. Did you just remove the ignore setting from >> .gitmodules or the path too? > > Just the ignore, and my patch of course. > > If it is not set in $GIT_DIR/config, then config_name_for_path is not > initialized, and if it is not initialized, then > set_diffopt_flags_from_submodule_config does nothing > (handle_ignore_submodules_arg is not called). That is the main > problem. But config_name_for_path can only be set via .gitmodules. It is set in parse_submodule_config_option() called by submodule_config() which is called from gitmodules_config() ... but only if .gitmodules doesn't have a merge conflict. So either your .gitmodules has a merge conflict or the logic setting gitmodules_is_unmerged in gitmodules_config() is buggy. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] commit: configure submodules 2012-09-24 19:56 ` Jens Lehmann 2012-09-24 19:59 ` Orgad Shaneh @ 2012-09-24 20:36 ` Junio C Hamano 1 sibling, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2012-09-24 20:36 UTC (permalink / raw) To: Jens Lehmann; +Cc: Orgad Shaneh, git Jens Lehmann <Jens.Lehmann@web.de> writes: > But submodule.mod.path should only be set in .gitmodules, not in > $GIT_DIR/config. Did you just remove the ignore setting from > .gitmodules or the path too? Without that in $GIT_DIR/config, how would path<->name mapping correctly work??? Confused... ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-09-24 20:36 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-09-23 7:37 [PATCH] commit: configure submodules Orgad Shaneh 2012-09-23 8:37 ` Junio C Hamano 2012-09-23 18:13 ` Jens Lehmann 2012-09-24 5:57 ` Junio C Hamano 2012-09-24 16:27 ` Junio C Hamano 2012-09-24 18:15 ` Jens Lehmann 2012-09-24 18:34 ` Junio C Hamano 2012-09-24 18:49 ` Orgad Shaneh 2012-09-24 19:06 ` Junio C Hamano 2012-09-24 19:16 ` Orgad Shaneh 2012-09-24 19:56 ` Jens Lehmann 2012-09-24 19:59 ` Orgad Shaneh 2012-09-24 20:10 ` Jens Lehmann 2012-09-24 20:36 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).