Re: [PATCH] commit: configure submodules

2012-09-24 Thread Junio C Hamano
Jens Lehmann  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...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: configure submodules

2012-09-24 Thread Jens Lehmann
Am 24.09.2012 21:59, schrieb Orgad Shaneh:
> On Mon, Sep 24, 2012 at 9:56 PM, Jens Lehmann  wrote:
>> Am 24.09.2012 21:16, schrieb Orgad Shaneh:
>>> On Mon, Sep 24, 2012 at 9:06 PM, Junio C Hamano  wrote:
 Orgad Shaneh  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 ..." to unstage)
>>> #
>>> #   modified:   foo
>>> #
>>> # Changes not staged for commit:
>>> #   (use "git add ..." to update what will be committed)
>>> #   (use "git checkout -- ..." 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 ..." to unstage)
>>> #
>>> #   modified:   foo
>>> #
>>> # Changes not staged for commit:
>>> #   (use "git add ..." to update what will be committed)
>>> #   (use "git checkout -- ..." 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: configure submodules

2012-09-24 Thread Orgad Shaneh
On Mon, Sep 24, 2012 at 9:56 PM, Jens Lehmann  wrote:
> Am 24.09.2012 21:16, schrieb Orgad Shaneh:
>> On Mon, Sep 24, 2012 at 9:06 PM, Junio C Hamano  wrote:
>>> Orgad Shaneh  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 ..." to unstage)
>> #
>> #   modified:   foo
>> #
>> # Changes not staged for commit:
>> #   (use "git add ..." to update what will be committed)
>> #   (use "git checkout -- ..." 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 ..." to unstage)
>> #
>> #   modified:   foo
>> #
>> # Changes not staged for commit:
>> #   (use "git add ..." to update what will be committed)
>> #   (use "git checkout -- ..." 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: configure submodules

2012-09-24 Thread Jens Lehmann
Am 24.09.2012 21:16, schrieb Orgad Shaneh:
> On Mon, Sep 24, 2012 at 9:06 PM, Junio C Hamano  wrote:
>> Orgad Shaneh  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 ..." to unstage)
> #
> #   modified:   foo
> #
> # Changes not staged for commit:
> #   (use "git add ..." to update what will be committed)
> #   (use "git checkout -- ..." 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 ..." to unstage)
> #
> #   modified:   foo
> #
> # Changes not staged for commit:
> #   (use "git add ..." to update what will be committed)
> #   (use "git checkout -- ..." 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: configure submodules

2012-09-24 Thread Orgad Shaneh
On Mon, Sep 24, 2012 at 9:06 PM, Junio C Hamano  wrote:
> Orgad Shaneh  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 ..." to unstage)
#
#   modified:   foo
#
# Changes not staged for commit:
#   (use "git add ..." to update what will be committed)
#   (use "git checkout -- ..." 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 ..." to unstage)
#
#   modified:   foo
#
# Changes not staged for commit:
#   (use "git add ..." to update what will be committed)
#   (use "git checkout -- ..." 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: configure submodules

2012-09-24 Thread Junio C Hamano
Orgad Shaneh  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?


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: configure submodules

2012-09-24 Thread Orgad Shaneh
On Mon, Sep 24, 2012 at 8:34 PM, Junio C Hamano  wrote:
>
> Jens Lehmann  writes:
>
> > Am 24.09.2012 18:27, schrieb Junio C Hamano:
> >> Junio C Hamano  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 
> 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 
> Signed-off-by: Junio C Hamano 
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: configure submodules

2012-09-24 Thread Junio C Hamano
Jens Lehmann  writes:

> Am 24.09.2012 18:27, schrieb Junio C Hamano:
>> Junio C Hamano  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 
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 
Signed-off-by: Junio C Hamano 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: configure submodules

2012-09-24 Thread Jens Lehmann
Am 24.09.2012 18:27, schrieb Junio C Hamano:
> Junio C Hamano  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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: configure submodules

2012-09-24 Thread Junio C Hamano
Junio C Hamano  writes:

> Jens Lehmann  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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: configure submodules

2012-09-23 Thread Junio C Hamano
Jens Lehmann  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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: configure submodules

2012-09-23 Thread Jens Lehmann
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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: configure submodules

2012-09-23 Thread Junio C Hamano
Orgad Shaneh  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 
> ---

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;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] commit: configure submodules

2012-09-23 Thread 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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html