Re: [RFC PATCH (WIP)] Show a dirty working tree and a detached HEAD in status for submodule

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jens Lehmann <Jens.Lehmann@xxxxxx> writes:

> Until now a submodule only showed up as changed in the supermodule when
> the last commit in the submodule differed from the one in the index or
> the last commit of the superproject. A dirty working tree or a detached
> HEAD in a submodule were just ignored when looking at it from the
> superproject.
>
> This patch shows these changes when using git status or one of the diff
> commands which compare against the working tree in the superproject.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@xxxxxx>
> ---
>
>
> This is the first version of a patch letting git status and the git
> diff family show dirty working directories and a detached HEAD in
> directories. It is not intended to be merged in its present form but
> to be used as a starting point for discussion if this is going in
> the right direction.
>
>
> What the patch does:
>
> * It makes git show submodules as modified in the superproject when
>   one or more of these conditions are met:
>
>     a) The submodule contains untracked files
>     b) The submodule contains modified files
>     c) The submodules HEAD is not on a local or remote branch
>
>   That can be seen when using either "git status", "git diff[-files]"
>   & "git diff[-index] HEAD" (and with "git gui" & gitk).

If the submodule is checked out, _and_ if the HEAD there, either detached
or not, does not agree with what the "other" one records (i.e. the commit
recorded in an entry in the index, or in the tree, that you are comparing
your work tree against), then it also should be considered modified.  I
don't think your (a)-(c) cover this case.

Also I don't understand why you want to treat (c) any specially at all.
Even if (c) is something we _should_ report, please do not call that as
"detached" in its implementation.  "detached HEAD" has a very precise
technical meaning, and can point at the same commit as a local or a remote
tracking branch, which is very different from the definition your
implementation seems to use.

> * This behavior is not configurable but activated by default. A config
>   option is needed here.

I doubt it.

My gut feeling is that this should be _always_ on for a submodule
directory that has been "submodule init/update".  The user is interested
in that particular submodule, and any change to it should be reported for
both classes of users.  Theose who meant to use the submodule read-only
need to be able to notice that they accidentally made the submodule dirty
before making a commit in the superproject.  Those who wanted to work in
submodule needs to know if the state is in sync with what they expect
before making a commit in the superproject.

That of course is provided if the unconditional check does not trigger for
submodules that the user hasn't "submodue init"ed; I think you did that
correctly at the beginning of your is_submodule_modified() implementation.

> +static int is_submodule_head_detached(const char *path)
> +{

I don't understand why you should care which branch the submodule happens
to be on, as long as the next commit you make in the superproject records
the commit that is checked out in the submodule.

Of course you may want to be careful when "pushing" the superproject
results out (i.e. you would want to push out the history leading to that
commit at the submodule HEAD in the submodule history), so that the people
who are pulling from the repository you are pushing into will have
everything available.

But the thing is, in a distributed environment, the submodule HEAD being
at the tip of _some_ branch (either local or remote) you have doesn't mean
anything to help them.  IOW, for protect others, you would need a check
when you _push out_ (either in 'push' or on the receiving end).

So I'd suggest dropping this condition in "status/diff" that is about
preparing to make the next commit in your _local_ history.

If "must be reachable from somewhere" is a condition worth caring about in
some context other than "status/diff", you can do an equivalent of:

    $ git rev-parse HEAD --not --all | git rev-list --stdin

and see if anything comes out (in which case you have commits that are not
reachable from any of your refs other than the detached HEAD).

But that is not "is HEAD detached?"; it is something else.  "Dangling",
perhaps, as that is how "git fsck" call commits that are not reachable
from any of your refs ("fsck" considers HEAD a part of refs, so it is not
strictly correct but it is much closer).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]