Re: Re: Git issues with submodules
On Mon, Nov 25, 2013 at 12:53:45PM -0800, Junio C Hamano wrote: Heiko Voigt hvo...@hvoigt.net writes: What I think needs fixing here first is that the ignore setting should not apply to any diffs between HEAD and index. IMO, it should only apply to the diff between worktree and index. Hmph. How about git diff $commit, the diff between the worktree and a named commit (which may or may not be HEAD)? Thats an interesting question. My first thought was that I would expect it to not show submodules since it involves the worktree, but then I could also argue that it should only show differences between whats in the index and the given commit. That would make matters more complicated but I image the use case (floating submodules) involves not caring about submodules except for some integration points when submodule sha1's are explicitly recorded. I would expect to only see diffs between these integration points. But then I am not a big user (none at all at the moment) of the floating model. Cheers Heiko -- 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: Git issues with submodules
Am 25.11.2013 22:01, schrieb Junio C Hamano: Jens Lehmann jens.lehm...@web.de writes: Looking good to me. Please add tests for diff.ignoreSubmodules and submodule.name.ignore, the latter both in .gitmodules and .git/config. While doing some testing for this thread I found an inconsistency in git show which currently honors the submodule specific option only from .git/config and ignores it in the .gitmodules file ... Sorry, but isn't that what should happen? .git/config is the ultimate source of the truth, and .gitmodules is a hint to prime that when the user does git submodule init, no? git submodule init only copies the update and url settings to .git/config, all others default to the value they have in the .gitmodules file if they aren't found in .git/config. This allows upstream to change these settings unless the user copies them to .git/config himself. -- 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: Git issues with submodules
Jens Lehmann jens.lehm...@web.de writes: Am 25.11.2013 22:01, schrieb Junio C Hamano: Jens Lehmann jens.lehm...@web.de writes: Looking good to me. Please add tests for diff.ignoreSubmodules and submodule.name.ignore, the latter both in .gitmodules and .git/config. While doing some testing for this thread I found an inconsistency in git show which currently honors the submodule specific option only from .git/config and ignores it in the .gitmodules file ... Sorry, but isn't that what should happen? .git/config is the ultimate source of the truth, and .gitmodules is a hint to prime that when the user does git submodule init, no? git submodule init only copies the update and url settings to .git/config, all others default to the value they have in the .gitmodules file if they aren't found in .git/config. This allows upstream to change these settings unless the user copies them to .git/config himself. I know what the code does. I was questioning if only copies X and Y is a sensible thing. Copying at init time will fix the values when copied and give the user a stable and dependable behaviour. I have a feeling that the current not copy to fix it to a stable value, but look into .gitmodules as a fallback was not a designed behaviour for the other properties, but was done by accident and/or laziness. -- 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: Git issues with submodules
Junio C Hamano wrote: I have a feeling that the current not copy to fix it to a stable value, but look into .gitmodules as a fallback was not a designed behaviour for the other properties, but was done by accident and/or laziness. It was designed. See for example the thread surrounding [1]: | And when you are on a superproject branch actively developing inside a | submodule, you may want to increase fetch-activity to fetch all new | commits in the submodule even if they aren't referenced in the | superproject (yet), as that might be just what your fellow developers | are about to do. And the person setting up that branch could do that | once for all users so they don't have to repeat it in every clone. And | when switching away from that branch all those developers cannot forget | to reconfigure to fetch-on-demand, so not having that in .git/config is | a plus here too. Thanks, Jonathan [1] http://thread.gmane.org/gmane.comp.version-control.git/161193/focus=161357 -- 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: Git issues with submodules
Jonathan Nieder jrnie...@gmail.com writes: Junio C Hamano wrote: I have a feeling that the current not copy to fix it to a stable value, but look into .gitmodules as a fallback was not a designed behaviour for the other properties, but was done by accident and/or laziness. It was designed. See for example the thread surrounding [1]: OK, thanks. | And when you are on a superproject branch actively developing inside a | submodule, you may want to increase fetch-activity to fetch all new | commits in the submodule even if they aren't referenced in the | superproject (yet), as that might be just what your fellow developers | are about to do. And the person setting up that branch could do that | once for all users so they don't have to repeat it in every clone. And | when switching away from that branch all those developers cannot forget | to reconfigure to fetch-on-demand, so not having that in .git/config is | a plus here too. Thanks, Jonathan [1] http://thread.gmane.org/gmane.comp.version-control.git/161193/focus=161357 -- 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: Git issues with submodules
Hey!
Sorry for the delayed reply.
Am i right the intention is to make it so `git add .` and `git commit
.` doesn't include changes to submodule hash unless -f argument is
provided?
On Sun, Nov 24, 2013 at 10:29 PM, Jens Lehmann jens.lehm...@web.de wrote:
Am 24.11.2013 01:52, schrieb Heiko Voigt:
Hi,
On Sat, Nov 23, 2013 at 09:10:44PM +0100, Jens Lehmann wrote:
Am 22.11.2013 23:09, schrieb Jonathan Nieder:
Heiko Voigt wrote:
After that we can discuss whether add should add submodules that are
tracked but not shown. How about commit -a ? Should it also ignore the
change? I am undecided here. There does not seem to be any good
decision. From the users point of view we should probably not add it
since its not visible in status. What do others think?
I agree --- it should not add.
I concur: adding a change that is hidden from the user during
the process is not a good idea.
Here is a patch achieving that. Still missing a test which I will add.
Looking good to me. Please add tests for diff.ignoreSubmodules
and submodule.name.ignore, the latter both in .gitmodules and
.git/config. While doing some testing for this thread I found an
inconsistency in git show which currently honors the submodule
specific option only from .git/config and ignores it in the
.gitmodules file (depending on the outcome of the discussion on
what '--ignore-submodules=all' should ignore we might have to fix
that one afterwards).
I'd suggest to also add the --ignore-submodules option in another
patch on top, because the user should be able to override the
configuration either way. And what about having the '-f' option
imply '--ignore-submodules=none'?
Cheers Heiko
---8
Subject: [PATCH] fix 'git add' to skip submodules configured as ignored
If submodules are configured as ignore=all they are not shown by status.
Lets also ignore them when adding files to the index. This avoids that
users accidentially add ignored submodules with: git add .
We achieve this by reading the submodule config and thus correctly
initializing the infrastructure to take the ignore decision.
Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
builtin/add.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/builtin/add.c b/builtin/add.c
index 226f758..2d0d2ef 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -15,6 +15,7 @@
#include diffcore.h
#include revision.h
#include bulk-checkin.h
+#include submodule.h
static const char * const builtin_add_usage[] = {
N_(git add [options] [--] pathspec...),
@@ -378,6 +379,10 @@ static int add_config(const char *var, const char
*value, void *cb)
ignore_add_errors = git_config_bool(var, value);
return 0;
}
+
+ if (!prefixcmp(var, submodule.))
+ return parse_submodule_config_option(var, value);
+
return git_default_config(var, value, cb);
}
@@ -415,6 +420,7 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
int implicit_dot = 0;
struct update_callback_data update_data;
+ gitmodules_config();
git_config(add_config, NULL);
argc = parse_options(argc, argv, prefix, builtin_add_options,
--
With best regards, Sergey Sharybin
--
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: Re: Git issues with submodules
On Mon, Nov 25, 2013 at 03:02:51PM +0600, Sergey Sharybin wrote: Am i right the intention is to make it so `git add .` and `git commit .` doesn't include changes to submodule hash unless -f argument is provided? Yes thats the goal. My patch currently only disables it when ignore is set to all. I will add another patch that implements the -f and --submodule-ignore option to both of them so the user has an easy way to bypass that. But having said that we changing existing behavior here so we have to investigate carefully whether we are not breaking peoples expectations (and script). That also applies to the other patch that enables showing them in diff and friends again. Cheers Heiko -- 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: Re: Git issues with submodules
Heiko, yeah sure see what you mean. Changing existing behavior is pretty PITA. Just one more question for now, are you referencing to the patch [RFC PATCH] disable complete ignorance of submodules for index - HEAD diff? Coz i tested it and seems it doesn't change behavior of add/commit. Also, i'm around to test the all patches which are related on submodules :) On Mon, Nov 25, 2013 at 11:49 PM, Heiko Voigt hvo...@hvoigt.net wrote: On Mon, Nov 25, 2013 at 03:02:51PM +0600, Sergey Sharybin wrote: Am i right the intention is to make it so `git add .` and `git commit .` doesn't include changes to submodule hash unless -f argument is provided? Yes thats the goal. My patch currently only disables it when ignore is set to all. I will add another patch that implements the -f and --submodule-ignore option to both of them so the user has an easy way to bypass that. But having said that we changing existing behavior here so we have to investigate carefully whether we are not breaking peoples expectations (and script). That also applies to the other patch that enables showing them in diff and friends again. Cheers Heiko -- With best regards, Sergey Sharybin -- 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: Re: Re: Git issues with submodules
On Mon, Nov 25, 2013 at 11:57:45PM +0600, Sergey Sharybin wrote: Heiko, yeah sure see what you mean. Changing existing behavior is pretty PITA. Just one more question for now, are you referencing to the patch [RFC PATCH] disable complete ignorance of submodules for index - HEAD diff? Coz i tested it and seems it doesn't change behavior of add/commit. Yep, that was just an RFC for status and diff. I think teaching add and commit to skip submodules if ignored are a separate topic and thus will be in a separate patch. I have to add tests and probably some more commands. The logic of ignoring submodules is implemented quite deep in the diff code. So changing it can affect quite some commands so we have to check quite carefully what will be affected and if we can change it without to much fallout. Also, i'm around to test the all patches which are related on submodules :) Thanks, good to know. Stay tuned! Cheers Heiko -- 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: Git issues with submodules
Heiko Voigt hvo...@hvoigt.net writes: What I think needs fixing here first is that the ignore setting should not apply to any diffs between HEAD and index. IMO, it should only apply to the diff between worktree and index. Hmph. How about git diff $commit, the diff between the worktree and a named commit (which may or may not be HEAD)? When we have that the user does not see the submodule changed when normally working. But after doing git add . the change to the submodule should be shown in status and diff regardless of the configuration. Yes, that sounds sensible. -- 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: Git issues with submodules
Jens Lehmann jens.lehm...@web.de writes: Looking good to me. Please add tests for diff.ignoreSubmodules and submodule.name.ignore, the latter both in .gitmodules and .git/config. While doing some testing for this thread I found an inconsistency in git show which currently honors the submodule specific option only from .git/config and ignores it in the .gitmodules file ... Sorry, but isn't that what should happen? .git/config is the ultimate source of the truth, and .gitmodules is a hint to prime that when the user does git submodule init, no? I'd suggest to also add the --ignore-submodules option in another patch on top, because the user should be able to override the configuration either way. And what about having the '-f' option imply '--ignore-submodules=none'? Yeah, this sudden change of semantics, which I think is going in the right direction in the longer run, does look like it may be robbing from those from the want specific revision, but not want to see the cruft in the top-level camp to pay those in the floating school. At least, with add -f, it allows people to add such ignored ones, just like you can git add -f cruft when cruft is not tracked and marked as ignored in the .gitignore mechansim. -- 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: Git issues with submodules
Am 24.11.2013 01:52, schrieb Heiko Voigt:
Hi,
On Sat, Nov 23, 2013 at 09:10:44PM +0100, Jens Lehmann wrote:
Am 22.11.2013 23:09, schrieb Jonathan Nieder:
Heiko Voigt wrote:
After that we can discuss whether add should add submodules that are
tracked but not shown. How about commit -a ? Should it also ignore the
change? I am undecided here. There does not seem to be any good
decision. From the users point of view we should probably not add it
since its not visible in status. What do others think?
I agree --- it should not add.
I concur: adding a change that is hidden from the user during
the process is not a good idea.
Here is a patch achieving that. Still missing a test which I will add.
Looking good to me. Please add tests for diff.ignoreSubmodules
and submodule.name.ignore, the latter both in .gitmodules and
.git/config. While doing some testing for this thread I found an
inconsistency in git show which currently honors the submodule
specific option only from .git/config and ignores it in the
.gitmodules file (depending on the outcome of the discussion on
what '--ignore-submodules=all' should ignore we might have to fix
that one afterwards).
I'd suggest to also add the --ignore-submodules option in another
patch on top, because the user should be able to override the
configuration either way. And what about having the '-f' option
imply '--ignore-submodules=none'?
Cheers Heiko
---8
Subject: [PATCH] fix 'git add' to skip submodules configured as ignored
If submodules are configured as ignore=all they are not shown by status.
Lets also ignore them when adding files to the index. This avoids that
users accidentially add ignored submodules with: git add .
We achieve this by reading the submodule config and thus correctly
initializing the infrastructure to take the ignore decision.
Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
builtin/add.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/builtin/add.c b/builtin/add.c
index 226f758..2d0d2ef 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -15,6 +15,7 @@
#include diffcore.h
#include revision.h
#include bulk-checkin.h
+#include submodule.h
static const char * const builtin_add_usage[] = {
N_(git add [options] [--] pathspec...),
@@ -378,6 +379,10 @@ static int add_config(const char *var, const char
*value, void *cb)
ignore_add_errors = git_config_bool(var, value);
return 0;
}
+
+ if (!prefixcmp(var, submodule.))
+ return parse_submodule_config_option(var, value);
+
return git_default_config(var, value, cb);
}
@@ -415,6 +420,7 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
int implicit_dot = 0;
struct update_callback_data update_data;
+ gitmodules_config();
git_config(add_config, NULL);
argc = parse_options(argc, argv, prefix, builtin_add_options,
--
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: Git issues with submodules
Am 22.11.2013 23:09, schrieb Jonathan Nieder: Heiko Voigt wrote: After that we can discuss whether add should add submodules that are tracked but not shown. How about commit -a ? Should it also ignore the change? I am undecided here. There does not seem to be any good decision. From the users point of view we should probably not add it since its not visible in status. What do others think? I agree --- it should not add. I concur: adding a change that is hidden from the user during the process is not a good idea. That leaves the question of how to add explicitly. git add -f? git add --ignore-submodules=none? I suspect git add and git commit -a have to learn the --ignore-submodules option anyway if we go that route. There are points in time (e.g. releasing a new version or having run an expansive test successfully) where some users want to update the submodules that are normally ignored to record the exact versions involved. -- 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: Git issues with submodules
Am 22.11.2013 22:54, schrieb Heiko Voigt: What I think needs fixing here first is that the ignore setting should not apply to any diffs between HEAD and index. IMO, it should only apply to the diff between worktree and index. Not only that. It should also apply to diffs between commits/trees and work tree but not between commits/trees. The reason the ignore setting was added three years ago was to avoid expensive work tree operations when it was clear that either the information wasn't wanted or it took too much time to determine that. And I doubt you want to see modifications to submodules in your work tree when diffing against HEAD but not when diffing against the index. And this behavior happens to be just what the floating branch model needs too. I'm not sure there isn't a use case out there that also needs to silence diff friends regarding submodule changes between commits/trees and/or index too (even though I cannot come up with one at the moment). So I propose to add worktree as another value for the ignore option - which ignores submodule modifications in the work tree - and leave all as it is. -- 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: Re: Git issues with submodules
Hi,
On Sat, Nov 23, 2013 at 09:10:44PM +0100, Jens Lehmann wrote:
Am 22.11.2013 23:09, schrieb Jonathan Nieder:
Heiko Voigt wrote:
After that we can discuss whether add should add submodules that are
tracked but not shown. How about commit -a ? Should it also ignore the
change? I am undecided here. There does not seem to be any good
decision. From the users point of view we should probably not add it
since its not visible in status. What do others think?
I agree --- it should not add.
I concur: adding a change that is hidden from the user during
the process is not a good idea.
Here is a patch achieving that. Still missing a test which I will add.
Cheers Heiko
---8
Subject: [PATCH] fix 'git add' to skip submodules configured as ignored
If submodules are configured as ignore=all they are not shown by status.
Lets also ignore them when adding files to the index. This avoids that
users accidentially add ignored submodules with: git add .
We achieve this by reading the submodule config and thus correctly
initializing the infrastructure to take the ignore decision.
Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
builtin/add.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/builtin/add.c b/builtin/add.c
index 226f758..2d0d2ef 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -15,6 +15,7 @@
#include diffcore.h
#include revision.h
#include bulk-checkin.h
+#include submodule.h
static const char * const builtin_add_usage[] = {
N_(git add [options] [--] pathspec...),
@@ -378,6 +379,10 @@ static int add_config(const char *var, const char *value,
void *cb)
ignore_add_errors = git_config_bool(var, value);
return 0;
}
+
+ if (!prefixcmp(var, submodule.))
+ return parse_submodule_config_option(var, value);
+
return git_default_config(var, value, cb);
}
@@ -415,6 +420,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int implicit_dot = 0;
struct update_callback_data update_data;
+ gitmodules_config();
git_config(add_config, NULL);
argc = parse_options(argc, argv, prefix, builtin_add_options,
--
1.8.5.rc3.1.gbe2a8c7
--
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: Re: Git issues with submodules
On Sat, Nov 23, 2013 at 09:32:45PM +0100, Jens Lehmann wrote: Am 22.11.2013 22:54, schrieb Heiko Voigt: What I think needs fixing here first is that the ignore setting should not apply to any diffs between HEAD and index. IMO, it should only apply to the diff between worktree and index. Not only that. It should also apply to diffs between commits/trees and work tree but not between commits/trees. The reason the ignore setting was added three years ago was to avoid expensive work tree operations when it was clear that either the information wasn't wanted or it took too much time to determine that. And I doubt you want to see modifications to submodules in your work tree when diffing against HEAD but not when diffing against the index. And this behavior happens to be just what the floating branch model needs too. I'm not sure there isn't a use case out there that also needs to silence diff friends regarding submodule changes between commits/trees and/or index too (even though I cannot come up with one at the moment). So I propose to add worktree as another value for the ignore option - which ignores submodule modifications in the work tree - and leave all as it is. I am not so sure about that. Only finding out what has changed (commit wise) in a submodule is expensive. Just finding out whether a submodule sha1 has changed is not expensive. Maybe we should completely stop respecting the ignore=all setting for history and diff between index and HEAD. AFAIK, we do not have any other setting that instruct git to ignore specific parts of the history unless explicitly asked for by specifying a pathspec. And I think a user should never miss by accident that something has changed in the repository. Cheers Heiko -- 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: Git issues with submodules
[+CC: Jens, the goto-guy for submodules] Sergey Sharybin wrote: Namely, `git ls-files -m` will show addons as modified, regardless ignore=all configuration. In the same time `git diff-index --name-only HEAD --` will show no changes at all. This happens because diff-index handles submodules explicitly (see diff-lib.c), while ls-files doesn't (see builtin/ls-files.c). My opinion is that this is a bug, and git ls-files needs to be taught to handle submodules properly. This leads to issues with Arcanist (which is a Phabricator's tool) who considers addons as uncommited changes and either complains on this or just adds this to commits. Does Arcanist use `git ls-files -m` to check? There're also some more issues which happens to our developers and which i can not quite reproduce. Do try to track down the other issues and let us know. Sometimes it happens so git checkout to another branch yields about uncommited changes to addons and doesn't checkout to another branch. I've seldom used submodules with branches, so I'll let others chime in. Cheers. -- 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: Git issues with submodules
Hey, Answers are inlined. On Fri, Nov 22, 2013 at 5:16 PM, Ramkumar Ramachandra artag...@gmail.com wrote: [+CC: Jens, the goto-guy for submodules] Sergey Sharybin wrote: Namely, `git ls-files -m` will show addons as modified, regardless ignore=all configuration. In the same time `git diff-index --name-only HEAD --` will show no changes at all. This happens because diff-index handles submodules explicitly (see diff-lib.c), while ls-files doesn't (see builtin/ls-files.c). My opinion is that this is a bug, and git ls-files needs to be taught to handle submodules properly. Shall i fire report somewhere or it's being handled by the folks reading this ML? This leads to issues with Arcanist (which is a Phabricator's tool) who considers addons as uncommited changes and either complains on this or just adds this to commits. Does Arcanist use `git ls-files -m` to check? Yes, Arcanist uses `git ls-files -m` to check whether there're local modifications. We might also contact phab developers asking to change it to `git diff --name-only HEAD --`. Is there a preferable way to get list of modified files and are this command intended to output the same results? There're also some more issues which happens to our developers and which i can not quite reproduce. Do try to track down the other issues and let us know. I'm trying, but doesn't happen here on laptop yet. Will give it another try (do have some ideas). Also directed our developers here who experienced the issue and might give some details, Sometimes it happens so git checkout to another branch yields about uncommited changes to addons and doesn't checkout to another branch. I've seldom used submodules with branches, so I'll let others chime in. Ok, thanks anyway :) -- With best regards, Sergey Sharybin -- 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: Git issues with submodules
Sergey Sharybin wrote: On Fri, Nov 22, 2013 at 5:16 PM, Ramkumar Ramachandra artag...@gmail.com wrote: [+CC: Jens, the goto-guy for submodules] Sergey Sharybin wrote: Namely, `git ls-files -m` will show addons as modified, regardless ignore=all configuration. In the same time `git diff-index --name-only HEAD --` will show no changes at all. This happens because diff-index handles submodules explicitly (see diff-lib.c), while ls-files doesn't (see builtin/ls-files.c). My opinion is that this is a bug, and git ls-files needs to be taught to handle submodules properly. Shall i fire report somewhere or it's being handled by the folks reading this ML? Bugs are reported and tackled on the list. This leads to issues with Arcanist (which is a Phabricator's tool) who considers addons as uncommited changes and either complains on this or just adds this to commits. Does Arcanist use `git ls-files -m` to check? Yes, Arcanist uses `git ls-files -m` to check whether there're local modifications. We might also contact phab developers asking to change it to `git diff --name-only HEAD --`. Is there a preferable way to get list of modified files and are this command intended to output the same results? I just checked it out: it uses `git ls-files -m` to get the list of unstaged changes; `git diff --name-only HEAD --` will list staged changes as well. -- 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: Git issues with submodules
On Fri, Nov 22, 2013 at 06:38:47PM +0530, Ramkumar Ramachandra wrote: Does Arcanist use `git ls-files -m` to check? Yes, Arcanist uses `git ls-files -m` to check whether there're local modifications. We might also contact phab developers asking to change it to `git diff --name-only HEAD --`. Is there a preferable way to get list of modified files and are this command intended to output the same results? I just checked it out: it uses `git ls-files -m` to get the list of unstaged changes; `git diff --name-only HEAD --` will list staged changes as well. That diff command compares the working tree and HEAD; if you are trying to match `ls-files -m`, you probably wanted just `git diff --name-only` to compare the working tree and the index. Although in a script you'd probably want to use the plumbing `git diff-files` instead. -Peff -- 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: Git issues with submodules
Ramkumar, not actually sure what you mean? For me `git diff --name-only HEAD --` ignores changes to submodules hash changes. Also apparently it became a known TODO for phabricator developers [1]. Jeff, kinda trying to match yes. Just don't want changes to submodules hash to be included. So, after all is it expected behavior of ls-files or not and if not shall i report it as a separate thread? :) [1] https://secure.phabricator.com/rARCe62b23e67deacc24469525cc5dea2b297a5073fb On Fri, Nov 22, 2013 at 9:11 PM, Jeff King p...@peff.net wrote: On Fri, Nov 22, 2013 at 06:38:47PM +0530, Ramkumar Ramachandra wrote: Does Arcanist use `git ls-files -m` to check? Yes, Arcanist uses `git ls-files -m` to check whether there're local modifications. We might also contact phab developers asking to change it to `git diff --name-only HEAD --`. Is there a preferable way to get list of modified files and are this command intended to output the same results? I just checked it out: it uses `git ls-files -m` to get the list of unstaged changes; `git diff --name-only HEAD --` will list staged changes as well. That diff command compares the working tree and HEAD; if you are trying to match `ls-files -m`, you probably wanted just `git diff --name-only` to compare the working tree and the index. Although in a script you'd probably want to use the plumbing `git diff-files` instead. -Peff -- With best regards, Sergey Sharybin -- 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: Git issues with submodules
Jeff King wrote: I just checked it out: it uses `git ls-files -m` to get the list of unstaged changes; `git diff --name-only HEAD --` will list staged changes as well. That diff command compares the working tree and HEAD; if you are trying to match `ls-files -m`, you probably wanted just `git diff --name-only` to compare the working tree and the index. Although in a script you'd probably want to use the plumbing `git diff-files` instead. Thanks for that. It's probably not worth fixing ls-files; I'll patch Arcanist to use diff-files instead. -- 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: Git issues with submodules
Sergey Sharybin wrote: Ramkumar, not actually sure what you mean? For me `git diff --name-only HEAD --` ignores changes to submodules hash changes. `git diff --name-only HEAD --` compares the worktree to HEAD (listing both staged and unstaged changes); we want `git diff --name-only --` to compare the worktree to the index (listing only unstaged changes), as Peff notes. Also apparently it became a known TODO for phabricator developers [1]. That was me :) So, after all is it expected behavior of ls-files or not and if not shall i report it as a separate thread? :) Actually, I doubt it's worth fixing ls-files. Your problem should be fixed when this is merged (hopefully in a few hours): https://github.com/facebook/arcanist/pull/121 Cheers. -- 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: Git issues with submodules
Ah, didn't notice you're the author of that pull-request Ramkumar :) So guess issue with arc can be considered solved now. But i'm still collecting more details about how to manage to commit change addons hash without arc command even (it happens to Campbell Barton really often). Will report back when we'll know something. On Fri, Nov 22, 2013 at 10:35 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Sergey Sharybin wrote: Ramkumar, not actually sure what you mean? For me `git diff --name-only HEAD --` ignores changes to submodules hash changes. `git diff --name-only HEAD --` compares the worktree to HEAD (listing both staged and unstaged changes); we want `git diff --name-only --` to compare the worktree to the index (listing only unstaged changes), as Peff notes. Also apparently it became a known TODO for phabricator developers [1]. That was me :) So, after all is it expected behavior of ls-files or not and if not shall i report it as a separate thread? :) Actually, I doubt it's worth fixing ls-files. Your problem should be fixed when this is merged (hopefully in a few hours): https://github.com/facebook/arcanist/pull/121 Cheers. -- With best regards, Sergey Sharybin -- 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: Git issues with submodules
Ok, got it now. To reproduce the issue: - Run git submodule update --recursive to make sure their SHA is changed. Then `git add /path/to/changed submodule` or just `git add .` - Modify any file from the parent repository - Neither of `git status`, `git diff` and `git diff-files --name-only` will show changes to a submodule, only changes to that file which was changed in parent repo. - Make a git commit. It will not list changes to submodule as wll. - `git show HEAD` will show changes to both file from and parent repository (which is expected) and will also show changes to the submodule hash (which is unexpected i'd say). On Fri, Nov 22, 2013 at 11:01 PM, Sergey Sharybin sergey@gmail.com wrote: Ah, didn't notice you're the author of that pull-request Ramkumar :) So guess issue with arc can be considered solved now. But i'm still collecting more details about how to manage to commit change addons hash without arc command even (it happens to Campbell Barton really often). Will report back when we'll know something. On Fri, Nov 22, 2013 at 10:35 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Sergey Sharybin wrote: Ramkumar, not actually sure what you mean? For me `git diff --name-only HEAD --` ignores changes to submodules hash changes. `git diff --name-only HEAD --` compares the worktree to HEAD (listing both staged and unstaged changes); we want `git diff --name-only --` to compare the worktree to the index (listing only unstaged changes), as Peff notes. Also apparently it became a known TODO for phabricator developers [1]. That was me :) So, after all is it expected behavior of ls-files or not and if not shall i report it as a separate thread? :) Actually, I doubt it's worth fixing ls-files. Your problem should be fixed when this is merged (hopefully in a few hours): https://github.com/facebook/arcanist/pull/121 Cheers. -- With best regards, Sergey Sharybin -- With best regards, Sergey Sharybin -- 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: Git issues with submodules
Sergey Sharybin wrote: To reproduce the issue: - Run git submodule update --recursive to make sure their SHA is changed. Then `git add /path/to/changed submodule` or just `git add .` - Modify any file from the parent repository - Neither of `git status`, `git diff` and `git diff-files --name-only` will show changes to a submodule, only changes to that file which was changed in parent repo. - Make a git commit. It will not list changes to submodule as wll. - `git show HEAD` will show changes to both file from and parent repository (which is expected) and will also show changes to the submodule hash (which is unexpected i'd say). Thanks Sergey; I can confirm that this is a bug. For some reason, the `git add .` is adding the ignored submodule to the index. After that, $ git diff-index @ is not showing the ignored submodule. Let me see if I can dig through this in greater detail. -- 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: Git issues with submodules
Am 22.11.2013 17:12, schrieb Ramkumar Ramachandra: Jeff King wrote: I just checked it out: it uses `git ls-files -m` to get the list of unstaged changes; `git diff --name-only HEAD --` will list staged changes as well. That diff command compares the working tree and HEAD; if you are trying to match `ls-files -m`, you probably wanted just `git diff --name-only` to compare the working tree and the index. Although in a script you'd probably want to use the plumbing `git diff-files` instead. Thanks for that. It's probably not worth fixing ls-files; I'll patch Arcanist to use diff-files instead. Good to have an short term solution for Sergey, but Heiko and I discussed this issue and agreed that we should fix ls-files. After all the user explicitly asked not to be bothered with submodule differences by configuring the ignore setting. -- 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: Git issues with submodules
Am 22.11.2013 19:11, schrieb Ramkumar Ramachandra: Sergey Sharybin wrote: To reproduce the issue: - Run git submodule update --recursive to make sure their SHA is changed. Then `git add /path/to/changed submodule` or just `git add .` - Modify any file from the parent repository - Neither of `git status`, `git diff` and `git diff-files --name-only` will show changes to a submodule, only changes to that file which was changed in parent repo. - Make a git commit. It will not list changes to submodule as wll. - `git show HEAD` will show changes to both file from and parent repository (which is expected) and will also show changes to the submodule hash (which is unexpected i'd say). Thanks Sergey; I can confirm that this is a bug. Hmm, looks like git show also needs to be fixed to honor the ignore setting from .gitmodules. It already does that for diff.ignoreSubmodules from either .git/config or git -c and also supports the --ignore-submodules command line option. The following fixes this inconsistency for me: --8--- diff --git a/builtin/log.c b/builtin/log.c index b708517..ca97cfb 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -25,6 +25,7 @@ #include version.h #include mailmap.h #include gpg-interface.h +#include submodule.h /* Set a default date-time format for git log (log.date config variable) */ static const char *default_date_mode = NULL; @@ -521,6 +522,7 @@ int cmd_show(int argc, const char **argv, const char *prefix int i, count, ret = 0; init_grep_defaults(); + gitmodules_config(); git_config(git_log_config, NULL); memset(match_all, 0, sizeof(match_all)); --8--- But the question is if that is the right thing to do: should diff.ignoreSubmodules and submodule.name.ignore only affect the diff family or also git log friends? That would make users blind for submodule history (which they already are when using diff friends, so that might be ok here too). For some reason, the `git add .` is adding the ignored submodule to the index. The ignore setting is documented to only affect diff output (including what checkout, commit and status show as modified). While I agree that this behavior is confusing for Sergey and not optimal for the floating branch model he uses, git is currently doing exactly what it should. And for people using the ignore setting to not having to stat submodules with huge and/or many files that behavior is what they want: don't bother me with what changed, but commit what I did change on purpose. We may have to rethink what should happen for users of the floating branch model though. After that, $ git diff-index @ is not showing the ignored submodule. Of course it isn't, it's configured not to. You'll have to use --ignore-submodules=dirty to override the configuration to make it show differences in the recorded hash. -- 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: Git issues with submodules
For some reason, the `git add .` is adding the ignored submodule to the index. The ignore setting is documented to only affect diff output (including what checkout, commit and status show as modified). While I agree that this behavior is confusing for Sergey and not optimal for the floating branch model he uses, git is currently doing exactly what it should. And for people using the ignore setting to not having to stat submodules with huge and/or many files that behavior is what they want: don't bother me with what changed, but commit what I did change on purpose. We may have to rethink what should happen for users of the floating branch model though. I totally see what's happening here and indeed current logic of `git add .` agree is correct from how it was designed to. I could also see why it might be useful to keep `git add .` and `git commit .` not to respect submodule ignore flag. The only confusing thing here is that if i stage changed submodule with this command i wouldn't see this submodule in changes to be committed wen doing a commit. So seems it's just matter of better communication of what's gonna to be committed in changes to be committed section? Or maybe even make it so `git status` will show staged changes from submdules hash regardless ignore flag? Just an ideas how to make communication what's going on a bit better :) And for sure don't think suppressing stuff from git show is a nice idea (if i understand your proposal f making submodule ignore option affect on other commands). -- With best regards, Sergey Sharybin -- 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: Re: Git issues with submodules
Hi, On Fri, Nov 22, 2013 at 10:01:44PM +0100, Jens Lehmann wrote: Hmm, looks like git show also needs to be fixed to honor the ignore setting from .gitmodules. It already does that for diff.ignoreSubmodules from either .git/config or git -c and also supports the --ignore-submodules command line option. The following fixes this inconsistency for me: --8--- diff --git a/builtin/log.c b/builtin/log.c index b708517..ca97cfb 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -25,6 +25,7 @@ #include version.h #include mailmap.h #include gpg-interface.h +#include submodule.h /* Set a default date-time format for git log (log.date config variable) */ static const char *default_date_mode = NULL; @@ -521,6 +522,7 @@ int cmd_show(int argc, const char **argv, const char *prefix int i, count, ret = 0; init_grep_defaults(); + gitmodules_config(); git_config(git_log_config, NULL); memset(match_all, 0, sizeof(match_all)); --8--- But the question is if that is the right thing to do: should diff.ignoreSubmodules and submodule.name.ignore only affect the diff family or also git log friends? That would make users blind for submodule history (which they already are when using diff friends, so that might be ok here too). For some reason, the `git add .` is adding the ignored submodule to the index. The ignore setting is documented to only affect diff output (including what checkout, commit and status show as modified). While I agree that this behavior is confusing for Sergey and not optimal for the floating branch model he uses, git is currently doing exactly what it should. And for people using the ignore setting to not having to stat submodules with huge and/or many files that behavior is what they want: don't bother me with what changed, but commit what I did change on purpose. We may have to rethink what should happen for users of the floating branch model though. This gets more nasty. When using 'git add .' you secretly add the submodule to the index. But it is neither shown in status nor diff --cached. commit actually complains there is nothing to add. But then once you add a local file to the index you can commit and secretly take the submodule change with you. What I think needs fixing here first is that the ignore setting should not apply to any diffs between HEAD and index. IMO, it should only apply to the diff between worktree and index. When we have that the user does not see the submodule changed when normally working. But after doing git add . the change to the submodule should be shown in status and diff regardless of the configuration. I will have a look at that. After that we can discuss whether add should add submodules that are tracked but not shown. How about commit -a ? Should it also ignore the change? I am undecided here. There does not seem to be any good decision. From the users point of view we should probably not add it since its not visible in status. What do others think? Cheers Heiko -- 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: Re: Git issues with submodules
Heiko Voigt wrote: After that we can discuss whether add should add submodules that are tracked but not shown. How about commit -a ? Should it also ignore the change? I am undecided here. There does not seem to be any good decision. From the users point of view we should probably not add it since its not visible in status. What do others think? I agree --- it should not add. That leaves the question of how to add explicitly. git add -f? git add --ignore-submodules=none? Thanks, Jonathan -- 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: Git issues with submodules
Jens Lehmann wrote: But the question is if that is the right thing to do: should diff.ignoreSubmodules and submodule.name.ignore only affect the diff family or also git log friends? That would make users blind for submodule history (which they already are when using diff friends, so that might be ok here too). No, I think it's the wrong thing to do. We don't want to show false history. The ignore setting is documented to only affect diff output (including what checkout, commit and status show as modified). While I agree that this behavior is confusing for Sergey and not optimal for the floating branch model he uses, git is currently doing exactly what it should. And for people using the ignore setting to not having to stat submodules with huge and/or many files that behavior is what they want: don't bother me with what changed, but commit what I did change on purpose. We may have to rethink what should happen for users of the floating branch model though. I'd argue that the only reason the diff-family is blind is because the commit hash changes in the first place; if the hash didn't change (ie. floating submodules were represented by 0s hash or something), we wouldn't have this problem. The correct solution is to also make `git add' blind. -- 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: Re: Git issues with submodules
Heiko Voigt wrote: What I think needs fixing here first is that the ignore setting should not apply to any diffs between HEAD and index. IMO, it should only apply to the diff between worktree and index. When we have that the user does not see the submodule changed when normally working. But after doing git add . the change to the submodule should be shown in status and diff regardless of the configuration. Yeah, I think this is a good direction. After that we can discuss whether add should add submodules that are tracked but not shown. How about commit -a ? Should it also ignore the change? Here, I think ignored submodules should behave like files matched by .gitignore: add should not add (`add -f` would be a good way to force it), and `commit -a` should also exclude it. -- 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
Git issues with submodules
Hey everyone from Blender developers! As you might already know, we've recently switched from SVN to Git to host Blender sources. In general it works really awesome, but we've got some issues with submodules. in SVN we had separate repositories for addons and translations which were attached to main tree as svn:external. The reason for this was: 1. Separate commit access between core sources and addons so nobody accidentally breaks anything in the core. 2. Separate commit history to help tracking issues down. For the most developers and all artists (yes, we've got loads of artists who builds blender on their own) it makes sense to always checkout latest versions of addons and translations when updating working tree. We used Git submodules as a replacement for svn:external, with some tweaks and specific of update procedure. Namely, we always do `git submodule update --remote` to pull all the latest changes from submodules. This will mark checkout as modified because submodule hash changes. To avoid infinite commits of submodule hash we've added ignore=all to their configuration. In most cases it works fine, but there're some circumstances when it gives weirdo issues. Namely, `git ls-files -m` will show addons as modified, regardless ignore=all configuration. In the same time `git diff-index --name-only HEAD --` will show no changes at all. This leads to issues with Arcanist (which is a Phabricator's tool) who considers addons as uncommited changes and either complains on this or just adds this to commits. This issue i might easily reproduce on my laptop with latest Git 1.8.4.3. There're also some more issues which happens to our developers and which i can not quite reproduce. Sometimes it happens so git checkout to another branch yields about uncommited changes to addons and doesn't checkout to another branch. My guess here is that submodule hash in master and branch was different and having hash modified in master somehow prevented changes from another branch to be checked out. In this case question would be: what would be the proper way to checkout branches when having submodules configured this way? Second issue is that some developers still manages to commit changes to submodule hash, which i have totally no idea why Git allows to include such a changes. I could not do such a commits on purpose even. Here're some links to help understanding what's going on: - Blender repository browser: http://developer.blender.org/diffusion/B/ - Task in our tracker about issues we've got with Git: http://developer.blender.org/T37528 - History of changes to addons hash: http://developer.blender.org/diffusion/B/history/master/release/scripts/addons We're totally new to git submodules and clarification (and maybe even confirmed bug with ls-files -m :) would really be appreciated. We're also open for suggestions about re-configuring our submodules so they works in a way we'd expect this. Thanks in advance! -- With best regards, Sergey Sharybin -- 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
