Re: [Bug report] 'git status' always says "Your branch is up-to-date with 'origin/master'"

2014-01-05 Thread Jiang Xin
2014/1/5 Thomas Ackermann :
> Since f223459 "status: always show tracking branch even no change"
> 'git status' (and 'git checkout master' always says
> "Your branch is up-to-date with 'origin/master'"
> even if 'origin/master' is way ahead from local 'master'.

Hi, Thomas

Can you provide your operations so that I can reproduce this issue?

In the commit you mentioned above, there was a new test case named
'checkout (up-to-date with upstream)' added in 't6040'. I also add two
test-cases locally in order to reproduce the issue you report, and run
them in arbitrary orders, but they all look fine:

ok 4 - checkout (behind upstream)
ok 5 - checkout (ahead upstream)
ok 6 - checkout (diverged from upstream)
ok 7 - checkout with local tracked branch
ok 8 - checkout (upstream is gone)
ok 9 - checkout (up-to-date with upstream)
ok 10 - checkout (upstream is gone)
ok 11 - checkout with local tracked branch
ok 12 - checkout (diverged from upstream)
ok 13 - checkout (ahead upstream)
ok 14 - checkout (behind upstream)
ok 15 - checkout (diverged from upstream)
ok 16 - checkout (upstream is gone)
ok 17 - checkout (ahead upstream)
ok 18 - checkout with local tracked branch
ok 19 - checkout (behind upstream)


The two additional test cases I used locally are:

checkout_test1() {
test_expect_success 'checkout (behind upstream)' '
(
cd test && git checkout b3
) >actual &&
test_i18ngrep "is behind .* by 1 commit, and can be
fast-forwarded" actual
'
}

checkout_test_2() {
test_expect_success 'checkout (ahead upstream)' '
(
cd test && git checkout b4
) >actual &&
test_i18ngrep "is ahead of .* by 2 commits" actual
'
}

-- 
Jiang Xin
--
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 PULL] l10n update for maint branch

2014-01-05 Thread Jiang Xin
Hi, Junio

Please pull l10n update for maint branch. It can be merged to master
without conflict.

The following changes since commit 5512ac5840c8bcaa487806cf402ff960091ab244:

  Git 1.8.5.2 (2013-12-17 11:42:12 -0800)

are available in the git repository at:

  git://github.com/git-l10n/git-po maint

for you to fetch changes up to cb0553651d9bbfc7ecdb9ebe8365a449156f3455:

  l10n: de.po: fix translation of 'prefix' (2014-01-03 18:21:38 +0100)


Ralf Thielow (1):
  l10n: de.po: fix translation of 'prefix'

 po/de.po | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

2014/1/4 Ralf Thielow :
> Hi Xin,
>
> please pull this fix of German translation for 'maint' branch.
> I assume Junio will synchronize 'maint' with 'master' that I don't
> have to merge it myself. The commit can be merged without
> conflicts.
>

--
Jiang Xin
--
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: [RFC v2] submodule: Respect requested branch on all clones

2014-01-05 Thread W. Trevor King
On Sun, Jan 05, 2014 at 04:33:14PM -0800, W. Trevor King wrote:
> The only people who would need *automatic* rebase recovery would be
> superproject devs update-cloning the subproject.  That's a small
> enough cross-section that I don't think it deserves the ambiguity of
> gitlink-to-reference.  In that case, all you really need is a way to
> force a recovery gitlink (i.e. add a 'commit' object to the tree by
> hand).

Actually, you recovering by hand is a lot easier.  Setup a
rebased-away gitlink target:

  mkdir subproject &&
  (
cd subproject &&
git init
echo 'Subproject' > README &&
git add README &&
git commit -m 'Subproject v1' &&
echo 'Changes' >> README &&
git commit -am 'Subproject v2'
  ) &&
  mkdir superproject &&
  (
cd superproject &&
git init
git submodule add ../subproject &&
git commit -m 'Superproject v1'
  ) &&
  (
cd subproject &&
git reset --hard HEAD^ &&
git reflog expire --expire=now --all &&
git gc --aggressive --prune=now
  )

Then a recursive clone of the superproject dies:

  $ git clone --recursive superproject super2
  Cloning into 'super2'...
  done.
  Submodule 'subproject' (/tmp/x/subproject) registered for path 'subproject'
  Cloning into 'subproject'...
  done.
  fatal: reference is not a tree: f589144d16282d1a80d17a9032c6f1d332e38dd0
  Unable to checkout 'f589144d16282d1a80d17a9032c6f1d332e38dd0' in submodule 
path 'subproject'

But you still have the submodule checkout (up until the $sha1 setup):

  $ cd super2
  $ git diff
  diff --git a/subproject b/subproject
  index f589144..82d4553 16
  --- a/subproject
  +++ b/subproject
  @@ -1 +1 @@
  -Subproject commit f589144d16282d1a80d17a9032c6f1d332e38dd0
  +Subproject commit 82d4553fe437ae014f22bbc87a082c6d19e5d9f9-dirty

And you can automatically update to match the upstream remote:

  $ git submodule update --remote --force
  Submodule path 'subproject': checked out 
'82d4553fe437ae014f22bbc87a082c6d19e5d9f9'
  $ git diff
  diff --git a/subproject b/subproject
  index f589144..82d4553 16
  --- a/subproject
  +++ b/subproject
  @@ -1 +1 @@
  -Subproject commit f589144d16282d1a80d17a9032c6f1d332e38dd0
  +Subproject commit 82d4553fe437ae014f22bbc87a082c6d19e5d9f9

When explicitly updating to the superproject or subproject's
(--remote) new tip is so easy, I don't see a need for floating the
gitlinks themselves.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


Re: [RFC v2] submodule: Respect requested branch on all clones

2014-01-05 Thread W. Trevor King
On Sun, Jan 05, 2014 at 03:39:43PM -0800, W. Trevor King wrote:
> On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote:
> > The reason why one would set a branch option here is to share the
> > superproject branch with colleagues. He can make sure they can
> > always fetch and checkout the submodule even though the branch there
> > is still under cleanup and thus will be rebased often. The commit
> > referenced by sha1 would not be available to a developer fetching
> > after a rebase.
> 
> Yeah, floating gitlinks are something else.  I'd be happy to have
> that functionality (gitlinks pointing to references) be built into
> gitlinks themselves, not added as an additional layer in the
> submodule script.  This "gitlinked sha1 rebased out of existence"
> scenario is the first I've heard where I think gitlinked references
> would be useful.

On the other hand, if the subproject has such a rebase, a superproject
dev can hop into an existing checkout, update around the rebase, add a
superproject commit with the fixed sha1, and push that for other
superproject devs.  The only people who would need *automatic* rebase
recovery would be superproject devs update-cloning the subproject.
That's a small enough cross-section that I don't think it deserves the
ambiguity of gitlink-to-reference.  In that case, all you really need
is a way to force a recovery gitlink (i.e. add a 'commit' object to
the tree by hand).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


Re: [PATCH 2/2] shallow: Remove unused code

2014-01-05 Thread Duy Nguyen
On Mon, Jan 6, 2014 at 7:00 AM, Ramsay Jones  wrote:
>
> Commit 58babfff ("shallow.c: the 8 steps to select new commits for
> .git/shallow", 05-12-2013) added a function to implement step 5 of
> the quoted eight steps, namely 'remove_nonexistent_ours_in_pack()'.
> This function implements an optional optimization step in the new
> shallow commit selection algorithm. However, this function has no
> callers. (The commented out call sites would need to change, in
> order to provide information required by the function.)
>
> Signed-off-by: Ramsay Jones 
> ---
>
> Hi Junio,
>
> This should, perhaps, be marked as RFC; if the patch to actually
> use this function is coming soon, then this is not needed.
> However, I suspect it could be slow in coming ... :-P

I'm not against this patch. I think the function is to demonstrate
what step 5 is, which is already in the history (unless there are
major errors that I'll need to resend the whole series). I may do the
real step 5 differently anyway to fit better how index-pack and
unpack-objects are run. Not fully realized yet.

>
> ATB,
> Ramsay Jones
>
>  builtin/receive-pack.c |  1 -
>  commit.h   |  2 --
>  fetch-pack.c   |  1 -
>  shallow.c  | 16 
>  4 files changed, 20 deletions(-)
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index de869b5..85bba35 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -1059,7 +1059,6 @@ static void update_shallow_info(struct command 
> *commands,
> struct command *cmd;
> int *ref_status;
> remove_nonexistent_theirs_shallow(si);
> -   /* XXX remove_nonexistent_ours_in_pack() */
> if (!si->nr_ours && !si->nr_theirs) {
> shallow_update = 0;
> return;
> diff --git a/commit.h b/commit.h
> index 5507460..30598c6 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -230,8 +230,6 @@ struct shallow_info {
>  extern void prepare_shallow_info(struct shallow_info *, struct sha1_array *);
>  extern void clear_shallow_info(struct shallow_info *);
>  extern void remove_nonexistent_theirs_shallow(struct shallow_info *);
> -extern void remove_nonexistent_ours_in_pack(struct shallow_info *,
> -   struct packed_git *);
>  extern void assign_shallow_commits_to_refs(struct shallow_info *info,
>uint32_t **used,
>int *ref_status);
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 1d0328d..d52de74 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -984,7 +984,6 @@ static void update_shallow(struct fetch_pack_args *args,
> return;
>
> remove_nonexistent_theirs_shallow(si);
> -   /* XXX remove_nonexistent_ours_in_pack() */
> if (!si->nr_ours && !si->nr_theirs)
> return;
> for (i = 0; i < nr_sought; i++)
> diff --git a/shallow.c b/shallow.c
> index f48f732..bbc98b5 100644
> --- a/shallow.c
> +++ b/shallow.c
> @@ -358,22 +358,6 @@ void remove_nonexistent_theirs_shallow(struct 
> shallow_info *info)
> info->nr_theirs = dst;
>  }
>
> -/* Step 5, remove non-existent ones in "ours" in the pack */
> -void remove_nonexistent_ours_in_pack(struct shallow_info *info,
> -struct packed_git *p)
> -{
> -   unsigned char (*sha1)[20] = info->shallow->sha1;
> -   int i, dst;
> -   trace_printf_key(TRACE_KEY, "shallow: 
> remove_nonexistent_ours_in_pack\n");
> -   for (i = dst = 0; i < info->nr_ours; i++) {
> -   if (i != dst)
> -   info->ours[dst] = info->ours[i];
> -   if (find_pack_entry_one(sha1[info->ours[i]], p))
> -   dst++;
> -   }
> -   info->nr_ours = dst;
> -}
> -
>  define_commit_slab(ref_bitmap, uint32_t *);
>
>  struct paint_info {
> --
> 1.8.5



-- 
Duy
--
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 2/2] shallow: Remove unused code

2014-01-05 Thread Ramsay Jones

Commit 58babfff ("shallow.c: the 8 steps to select new commits for
.git/shallow", 05-12-2013) added a function to implement step 5 of
the quoted eight steps, namely 'remove_nonexistent_ours_in_pack()'.
This function implements an optional optimization step in the new
shallow commit selection algorithm. However, this function has no
callers. (The commented out call sites would need to change, in
order to provide information required by the function.)

Signed-off-by: Ramsay Jones 
---

Hi Junio,

This should, perhaps, be marked as RFC; if the patch to actually
use this function is coming soon, then this is not needed.
However, I suspect it could be slow in coming ... :-P

ATB,
Ramsay Jones

 builtin/receive-pack.c |  1 -
 commit.h   |  2 --
 fetch-pack.c   |  1 -
 shallow.c  | 16 
 4 files changed, 20 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index de869b5..85bba35 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1059,7 +1059,6 @@ static void update_shallow_info(struct command *commands,
struct command *cmd;
int *ref_status;
remove_nonexistent_theirs_shallow(si);
-   /* XXX remove_nonexistent_ours_in_pack() */
if (!si->nr_ours && !si->nr_theirs) {
shallow_update = 0;
return;
diff --git a/commit.h b/commit.h
index 5507460..30598c6 100644
--- a/commit.h
+++ b/commit.h
@@ -230,8 +230,6 @@ struct shallow_info {
 extern void prepare_shallow_info(struct shallow_info *, struct sha1_array *);
 extern void clear_shallow_info(struct shallow_info *);
 extern void remove_nonexistent_theirs_shallow(struct shallow_info *);
-extern void remove_nonexistent_ours_in_pack(struct shallow_info *,
-   struct packed_git *);
 extern void assign_shallow_commits_to_refs(struct shallow_info *info,
   uint32_t **used,
   int *ref_status);
diff --git a/fetch-pack.c b/fetch-pack.c
index 1d0328d..d52de74 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -984,7 +984,6 @@ static void update_shallow(struct fetch_pack_args *args,
return;
 
remove_nonexistent_theirs_shallow(si);
-   /* XXX remove_nonexistent_ours_in_pack() */
if (!si->nr_ours && !si->nr_theirs)
return;
for (i = 0; i < nr_sought; i++)
diff --git a/shallow.c b/shallow.c
index f48f732..bbc98b5 100644
--- a/shallow.c
+++ b/shallow.c
@@ -358,22 +358,6 @@ void remove_nonexistent_theirs_shallow(struct shallow_info 
*info)
info->nr_theirs = dst;
 }
 
-/* Step 5, remove non-existent ones in "ours" in the pack */
-void remove_nonexistent_ours_in_pack(struct shallow_info *info,
-struct packed_git *p)
-{
-   unsigned char (*sha1)[20] = info->shallow->sha1;
-   int i, dst;
-   trace_printf_key(TRACE_KEY, "shallow: 
remove_nonexistent_ours_in_pack\n");
-   for (i = dst = 0; i < info->nr_ours; i++) {
-   if (i != dst)
-   info->ours[dst] = info->ours[i];
-   if (find_pack_entry_one(sha1[info->ours[i]], p))
-   dst++;
-   }
-   info->nr_ours = dst;
-}
-
 define_commit_slab(ref_bitmap, uint32_t *);
 
 struct paint_info {
-- 
1.8.5
--
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 1/2] send-pack.c: mark a file-local function static

2014-01-05 Thread Ramsay Jones

Commit f2c681cf ("send-pack: support pushing from a shallow clone
via http", 05-12-2013) adds the 'advertise_shallow_grafts_buf'
function as an external symbol.

Noticed by sparse. ("'advertise_shallow_grafts_buf' was not declared.
Should it be static?")

Signed-off-by: Ramsay Jones 
---
 send-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/send-pack.c b/send-pack.c
index 2a199cc..6129b0f 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -183,7 +183,7 @@ static int advertise_shallow_grafts_cb(const struct 
commit_graft *graft, void *c
return 0;
 }
 
-void advertise_shallow_grafts_buf(struct strbuf *sb)
+static void advertise_shallow_grafts_buf(struct strbuf *sb)
 {
if (!is_repository_shallow())
return;
-- 
1.8.5
--
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: [RFC v2] submodule: Respect requested branch on all clones

2014-01-05 Thread W. Trevor King
On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote:
> On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote:
> > If submodule..branch is set, it *always* creates a new local
> > branch of that name pointing to the exact sha1.  If
> > submodule..branch is not set, we still create a
> > detached-HEAD checkout of the exact sha1.
> 
> Thanks for this clarification. Since the usual usage with --remote
> is with a remote-tracking branch, I confused this here. I am not
> sure whether blindly creating a local branch from the recorded sha1
> is the right thing to do. In what situations would that be helpful?

In any situation where your going to develop the submodule locally,
you're going to want a branch to develop in.  Starting local-submodule
developers off on a branch seems useful, even if we can only use
submodule..branch to guess at their preferred local branch name.
Sometimes (often?) the guess will be right.  However, A detached HEAD
will never be right for local development, so being right sometimes is
still an improvement ;).

> At $dayjob we usually use feature branches for our work. So if
> someone wants to work in a submodule you simply create a branch at
> the current sha1 which you then send out for review.

I'm all for named feature branches for development, and in this case
submodule..branch is likely to be the wrong choice.  However,
it's still safer to develop in that branch and then rename the branch
to match your feature than it would be to develop your fix with a
detached HEAD.  If your developers have enough discipline to always
checkout their feature branch before starting development, my patch
won't affect them.  However, I know a number of folks who go into
fight-or-flight mode when they have a detached HEAD :p.

> The reason why one would set a branch option here is to share the
> superproject branch with colleagues. He can make sure they can
> always fetch and checkout the submodule even though the branch there
> is still under cleanup and thus will be rebased often. The commit
> referenced by sha1 would not be available to a developer fetching
> after a rebase.

Yeah, floating gitlinks are something else.  I'd be happy to have that
functionality (gitlinks pointing to references) should be built into
gitlinks themselves, not added as an additional layer in the submodule
script.  This "gitlinked sha1 rebased out of existence" scenario is
the first I've heard where I think gitlinked references would be
useful.

> > Thinking through this more, perhaps the logic should be:
> > 
> > * If submodule..update (defaulting to checkout) is checkout,
> >   create a detached HEAD.
> > * Otherwise, create a new branch submodule..branch
> >   (defaulting to master).
> 
> Why not trigger the attached state with the submodule..branch
> configuration option? If there is a local branch available use that,
> if not the tracking branch (as it is currently). Then a developer
> can start working on the branch with:
> 
>   cd submodule; git checkout -t origin/
> 
> assuming that submodule update learns some more support for this.

Isn't that already what 'git update --remote ' already
does?

> > > > -   update_module= ;;
> > > > +   if test -n "$config_branch"; then
> > > > +   update_module="!git reset 
> > > > --hard -q"
> > > 
> > > If we get here the checkout has already been done. Shouldn't
> > > this rather specify a noop. I.E. like
> > > 
> > >   update_module="!true"
> > 
> > We are on a local branch at this point, but not neccessarily
> > pointing at the gitlinked sha1.  The reset here ensures that the
> > new local branch does indeed point at the gitlinked sha1.
> 
> But isn't this a fresh clone? Why should the branch point at
> anything else?

We don't pass $sha1 to module_clone().  Before my patch, we don't even
pass $branch to module_clone().  That means that module_clone() will
only checkout the gitlinked sha1 when the upstream HEAD (or $branch
with my patch) happens to point to the gitlinked sha1.  For example,
if Alice adds Charie's repo as a submodule (gitlinking his current
master d2dbd39), then Charlie pushes a new commit d0de817 to his
master, and then Bob clones Alice's superproject.  Post-clone,
Charlie's submodule will have checked out Charlie's new d0de817, and
we need update's additional:

  git reset --hard -q d2dbd39

to rewind to Alice's gitlinked sha1.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


Re: [PATCH 2/2] Introduce git submodule attached update

2014-01-05 Thread Francesco Pretto
2014/1/5 Heiko Voigt :
> Could you please extend the description of your use-case so we can
> understand your goal better?
>

Maybe I found better words to explain you my goal: the current git
submodule use-case threats the submodule as a project independent
dependency. My use case threats the submodule as part of the
superproject repository. It could be easier to say that in this way
submodules would behave very similarly to "svn:externals", something
that is actually missing in git. My goal is obtain this without
altering git behavior for the existing use case.

>  - In which situations does the developer or maintainer switch between
>your attached/detached mode?

As I told you in the other answer this is voluntary done by the
developer, as he prefers. I came to the conclusion that the
"--attach|--detach" switches for the "update" command are not that
useful and can be removed. It's still possible to obtain the switch
between detached/attached very easily in this way:

# Attach submodule
$ git config submodule..attached "true"
$ git submodule update

# Detach submodule
$ git config submodule..attached "false"
$ git submodule update

# Unset property in both ".gitmodules" and ".git/config" means -> do nothing
$ git config --unset submodule..attached
$ git submodule update

Also my "submodule..attached" property at the moment behaves
like "submodule..update": it is copied in ".git/config" by "git
submodule init". This is probably a mistake: the overridden value
should be stored in ".git/config" only at the developer will, so the
maintainer has still a chance to modify it in ".gitmodules" and
propagate the behavior.

I would send an updated patch but at this point I prefer to wait for a
full review.

Thank you,
Francesco
--
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: [RFC v2] submodule: Respect requested branch on all clones

2014-01-05 Thread Heiko Voigt
On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote:
> On Sun, Jan 05, 2014 at 08:48:50PM +0100, Heiko Voigt wrote:
> > On Sun, Jan 05, 2014 at 08:17:00AM -0800, W. Trevor King wrote:
> > > It's not clear if this refers to the initial-clone update, future
> > > post-clone updates, or both.  Ideally, the behavior should be the
> > > same, but in the initial-clone case we don't have an existing
> > > checked-out branch to work with.
> > 
> > I do not think that its actually important to end up with a detached
> > HEAD. The documentation just states it because in most cases there
> > is no other option. But I do not think anything will break if a
> > branch points to the exact sha1 we would checkout and we checkout
> > the branch instead.
> 
> There's no "if the remote-tracking branch points to the exact sha1"
> logic in my patch.

I know I was more referring to the discussion whether detached HEAD for
submodules is important or not.

> If submodule..branch is set, it *always*
> creates a new local branch of that name pointing to the exact sha1.
> If submodule..branch is not set, we still create a detached-HEAD
> checkout of the exact sha1.

Thanks for this clarification. Since the usual usage with --remote is
with a remote-tracking branch, I confused this here. I am not sure
whether blindly creating a local branch from the recorded sha1 is the
right thing to do. In what situations would that be helpful?

At $dayjob we usually use feature branches for our work. So if someone
wants to work in a submodule you simply create a branch at the current
sha1 which you then send out for review.

The reason why one would set a branch option here is to share the
superproject branch with colleagues. He can make sure they can always
fetch and checkout the submodule even though the branch there is still
under cleanup and thus will be rebased often. The commit referenced by
sha1 would not be available to a developer fetching after a rebase.

> Thinking through this more, perhaps the
> logic should be:
> 
> * If submodule..update (defaulting to checkout) is checkout,
>   create a detached HEAD.
> * Otherwise, create a new branch submodule..branch (defaulting
>   to master).

Why not trigger the attached state with the submodule..branch
configuration option? If there is a local branch available use that, if
not the tracking branch (as it is currently). Then a developer can start
working on the branch with:

cd submodule; git checkout -t origin/

assuming that submodule update learns some more support for this.

> The motivation is that if submodule..update is checkout, the
> user is unlikely to be developing locally in the submodule, as
> subsequent updates would clobber their local commits.  Having a
> detached HEAD is a helpful "don't develop here" reminder ;).  If
> submodule..update is set, the user is likely to be developing
> locally, and will probably want a local branch already checked out to
> facilitate that.
> 
> > > - module_clone "$sm_path" "$name" "$url" "$reference" 
> > > "$depth" || exit
> > > + module_clone "$sm_path" "$name" "$url" "$reference" 
> > > "$depth" "$config_branch" || exit
> > 
> > In the simple case (update=checkout, no branch specified) with the
> > new checkout branch stuff in module_clone() this code here ends up
> > calling checkout twice.  First for master and then here later with
> > the sha1.  This feels a little bit double.
> 
> There is no guarantee that the remote master and the exact sha1 point
> at the same commit.  Ideally we'd just clone the exact sha1 in this
> case.
> 
> > I would prefer if we skip the checkout in module_clone() if its not
> > necessary.
> 
> When I tried to drop the '' case here:
> 
> > > @@ -306,7 +307,15 @@ module_clone()
> > >   echo "gitdir: $rel/$a" >"$sm_path/.git"
> > >  
> > >   rel=$(echo $a | sed -e 's|[^/][^/]*|..|g')
> > > - (clear_local_git_env; cd "$sm_path" && GIT_WORK_TREE=. git config 
> > > core.worktree "$rel/$b")
> > > + (
> > > + clear_local_git_env
> > > + cd "$sm_path" &&
> > > + GIT_WORK_TREE=. git config core.worktree "$rel/$b" &&
> > > + case "$branch" in
> > > + '') git checkout -f -q ;;
> > > + ?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
> > > + esac
> > > + ) || die "$(eval_gettext "Unable to setup cloned submodule 
> > > '\$sm_path'")"
> > >  }
> 
> I got test-suite errors that I didn't get to the bottom of.  However…
> 
> > How about we move the whole "what to checkout"-decision into one place
> > instead of having it in update() and moving it from add() into
> > module_clone() ?
> 
> …this sounds like a good idea to me.  However, it would be a more
> intrusive change, and there may be conflicts with Francesco's proposed
> attach/detach functionality.  I'll wait until we have a clearer idea
> of where that is headed before I attempt a more complete
> consolidation.

I agree, that would be good.

> > > -

Re: [RFC v2] submodule: Respect requested branch on all clones

2014-01-05 Thread W. Trevor King
On Sun, Jan 05, 2014 at 01:47:52PM -0800, W. Trevor King wrote:
> On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote:
> > 2014/1/5 W. Trevor King:
> > Also it covers:
> > - an "autoremote" behavior when the user wants an attached HEAD:
> > with your patch "--remote" is still needed to really update the
> > branch with "git submodule update":
> > - voluntary reattach/detach the HEAD with command line;
> > - ff-only merge of changes in the case of "checkout" in an attached
> > HEAD (doing "git checkout " is not enough);
> > - reattach of the HEAD with orphaned commits.
> 
> Personally, I don't think autoremote updates are worth the additional
> UI complication (hence my alternative patch ;), but I'm open to
> discussion on this point.  Can you make a case for why and explicit
> --remote update is burdensome?
> 
> I'm also not entirely clear on the problems avoided or workflows
> enhanced via the last two entries.  Could you sketch an example
> workflow that makes that more obvious?

For example, your original patch [1] claimed a reduction from:

  # Maintainer
  $ git submodule add --branch "master-project1"  common
  $ git commit -m "Added submodule"
  $ git config -f .gitmodules submodule.common.ignore all
  $ git push
  $ cd 
  $ git checkout "master-project1"

to:

  # Maintainer
  $ git submodule add --branch "master-project1" --attach  
  $ git commit -m "Added submodule"
  $ git push

My patch does not effect this maintainer flow at all, but I'm pretty
sure the initial checkout is already automatic:

  $ git --version
  git version 1.8.3.2
  $ cd b/
  $ git init
  Initialized empty Git repository in /tmp/b/.git/
  $ git submodule add --branch master ../a
  Cloning into 'a'...
  done.
  Checking connectivity... done
  $ cd a/
  $ git branch
  * master

You also claimed a reduction from:

  # Developer
  $ git pull
  $ git submodule init
  $ git submodule update --remote
  $ cd 
  $ branch="$(git config -f ..\.gitmodules submodule.common.branch)"; git 
checkout $branch

to:

  # Developer
  $ git pull
  $ git submodule init
  $ git submodule update

My patch should cover the developer reduction (auto branch checkout on
the initial cloning update) without confusing the situation with
autofloated updates.

Cheers,
Trevor

[1]: http://article.gmane.org/gmane.comp.version-control.git/239799

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


Re: [RFC v2] submodule: Respect requested branch on all clones

2014-01-05 Thread W. Trevor King
On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote:
> 2014/1/5 W. Trevor King:
> > Adding a check to only checkout submodule..branch if
> > submodule..update was 'rebase', 'merge', or 'none' would be
> > easy, but I don't think that makes much sense.  I can't see any
> > reason for folks who specify submodule..branch to prefer a
> > detached HEAD over a local branch matching the remote branch's
> > name.
> 
> I think the reason is that it still matches the original use case of
> submodules devs:
> - the maintainer decides the specific commit developers should have;
> - developers checkout that commit and don't pull (you can't do "git
> pull" in a detached HEAD);
> - they optionally get the upstream commit from the specified
> "submodule..branch" with "--remote". They are still in a
> detached HEAD and can't do "git pull".
> 
> Maybe who coded submodules at first was thinking that the best way to
> contribute to a project is to checkout that repository, and not work
> in the submodule. As said, this works well when the submodule
> repository is a full project, and not a bunch of shared code.

You're saying that the detached HEAD is a feature because it breaks
pull?  And developers can't be trusted/trained to just not pull
reflexively?  I'm not buying that ;).  Although I was sad to see
jc/pull-training-wheel dropped and have that discussion stall out [1].

> > If they prefer checkout updates, the first such update will return
> > them to a detached HEAD.  If they prefer merge/rebase updates,
> > future updates will keep them on the same branch.  All my commit
> > does is setup that initial branch for folks who get the submodule
> > via 'update', in the same way it's currently setup for folks who
> > get the submodule via 'add'.
> 
> My patch is in the same spirit but wants to obtain the same by being
> 100% additive and by not altering existing behavior in any way.

My v2 patch doesn't break the current test suite.  I'd be surprised if
a change in such peripheral existing behavior as the post clone-update
branch actually break any user code, but I'd be happy to see links
that prove me wrong.

> Also it covers:
> - an "autoremote" behavior when the user wants an attached HEAD:
> with your patch "--remote" is still needed to really update the
> branch with "git submodule update":
> - voluntary reattach/detach the HEAD with command line;
> - ff-only merge of changes in the case of "checkout" in an attached
> HEAD (doing "git checkout " is not enough);
> - reattach of the HEAD with orphaned commits.

Personally, I don't think autoremote updates are worth the additional
UI complication (hence my alternative patch ;), but I'm open to
discussion on this point.  Can you make a case for why and explicit
--remote update is burdensome?

I'm also not entirely clear on the problems avoided or workflows
enhanced via the last two entries.  Could you sketch an example
workflow that makes that more obvious?

> Unfortunately our patches are already a bit colliding. I'll wait for
> other comments from git maintainers and let see. Anyway, I'm happy
> because things are moving: after this debate git submodules will be
> better for sure.

+1.  I floated my patch as a proof-of-concept for my side of this
debate, but I'm pretty happy with the current setup, so it's hard for
me to imagine submodules getting worse as a result of this ;).

Cheers,
Trevor

[1]: http://article.gmane.org/gmane.comp.version-control.git/236372

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


Re: [PATCH 2/2] Introduce git submodule attached update

2014-01-05 Thread Francesco Pretto
2014/1/5 Heiko Voigt :
>
> Could you please extend the description of your use-case so we can
> understand your goal better?
>

Just in case you missed the first patch iteration[1].

> The following questions directly pop into my mind:
>
>  - What means the maintainer does not track the submodules sha1? Does
>that mean the superproject always refers to submodule commits using
>branches?

It means he doesn't need to control other developers commit to be
checked out so he sets "submodule..ignore" to "all". In this way
he and the developers can work actively in their submodule copy.

>  - What happens if you want to go back to an earlier revision? Lets say
>a tagged release? How is ensured that you get the correct revision in
>the submodules?

"submodule..branch" is one setting that is not copied in
".git/config" by "git submodule init". "git submodule update" will use
the setting in ".gitmodules" if not overridden voluntarily by the
developer in ".git/config". The maintainer can change that setting in
".gitmodules" and commit the change. Modifies will be propagated by
the next "git pull && git submodule update" of the developer in the
superproject.

>  - In which situations does the developer or maintainer switch between
>your attached/detached mode?

The developer/maintainer does so optionally and voluntarily and it
effects only its private working tree.

>  - What is the "repository branch" which is given to the developer by
>the maintainer used for? Who creates this branch and who merges into
>it?

The branch of course must exist prior submodule adding. In this
use-case it does not really matter who creates it and who merges into
it. Everyone with the right to merge into it has to work in the
submodule seamlessly, as it was working on separate clone of the same
repository used as the submodule.

>  - What are these subsequent "merge" or "rebase" update operations? Do
>you mean everyone has submodule.name.update configured to merge or
>rebase?
>

subsequent "merge" or "rebase" update operations are just the ones
after the initial clone/checkout, nothing particular.

Greetings,
Francesco

[1] http://marc.info/?l=git&m=138836829531511&w=2
--
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: [RFC v2] submodule: Respect requested branch on all clones

2014-01-05 Thread Francesco Pretto
2014/1/5 W. Trevor King :
> On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote:
>> Also it could break some users that rely on the current behavior.
>
> The current code always has a detached HEAD after an initial-clone
> update, regardless of submodule..update, which doesn't match
> those docs either.

I perfectly agree with you that the documentation is a bit
contradictory with regard to "update" command and detached HEAD.
That's why it's so hard to add a feature and keep the same spirit of
those that coded submodules at first. Also, I think that submodules
didn't get much feedback with regards to these pitfalls because many
people try to setup them, they see that "update" detaches the HEAD and
they think "hmmm, maybe submodules are not what I was looking for".

> Adding a check to only checkout
> submodule..branch if submodule..update was 'rebase',
> 'merge', or 'none' would be easy, but I don't think that makes much
> sense.  I can't see any reason for folks who specify
> submodule..branch to prefer a detached HEAD over a local branch
> matching the remote branch's name.

I think the reason is that it still matches the original use case of
submodules devs:
- the maintainer decides the specific commit developers should have;
- developers checkout that commit and don't pull (you can't do "git
pull" in a detached HEAD);
- they optionally get the upstream commit from the specified
"submodule..branch" with "--remote". They are still in a
detached HEAD and can't do "git pull".

Maybe who coded submodules at first was thinking that the best way to
contribute to a project is to checkout that repository, and not work
in the submodule. As said, this works well when the submodule
repository is a full project, and not a bunch of shared code.

> If they prefer checkout updates,
> the first such update will return them to a detached HEAD.  If they
> prefer merge/rebase updates, future updates will keep them on the same
> branch.  All my commit does is setup that initial branch for folks who
> get the submodule via 'update', in the same way it's currently setup
> for folks who get the submodule via 'add'.
>

My patch is in the same spirit but wants to obtain the same by being
100% additive and by not altering existing behavior in any way. Also
it covers:
- an "autoremote" behavior when the user wants an attached HEAD: with
your patch "--remote" is still needed to really update the branch with
"git submodule update":
- voluntary reattach/detach the HEAD with command line;
- ff-only merge of changes in the case of "checkout" in an attached
HEAD (doing "git checkout " is not enough);
- reattach of the HEAD with orphaned commits.

Unfortunately our patches are already a bit colliding. I'll wait for
other comments from git maintainers and let see. Anyway, I'm happy
because things are moving: after this debate git submodules will be
better for sure.

Cheers,
Francesco
--
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: [RFC v2] submodule: Respect requested branch on all clones

2014-01-05 Thread W. Trevor King
On Sun, Jan 05, 2014 at 08:48:50PM +0100, Heiko Voigt wrote:
> On Sun, Jan 05, 2014 at 08:17:00AM -0800, W. Trevor King wrote:
> > It's not clear if this refers to the initial-clone update, future
> > post-clone updates, or both.  Ideally, the behavior should be the
> > same, but in the initial-clone case we don't have an existing
> > checked-out branch to work with.
> 
> I do not think that its actually important to end up with a detached
> HEAD. The documentation just states it because in most cases there
> is no other option. But I do not think anything will break if a
> branch points to the exact sha1 we would checkout and we checkout
> the branch instead.

There's no "if the remote-tracking branch points to the exact sha1"
logic in my patch.  If submodule..branch is set, it *always*
creates a new local branch of that name pointing to the exact sha1.
If submodule..branch is not set, we still create a detached-HEAD
checkout of the exact sha1.  Thinking through this more, perhaps the
logic should be:

* If submodule..update (defaulting to checkout) is checkout,
  create a detached HEAD.
* Otherwise, create a new branch submodule..branch (defaulting
  to master).

The motivation is that if submodule..update is checkout, the
user is unlikely to be developing locally in the submodule, as
subsequent updates would clobber their local commits.  Having a
detached HEAD is a helpful "don't develop here" reminder ;).  If
submodule..update is set, the user is likely to be developing
locally, and will probably want a local branch already checked out to
facilitate that.

> > -   module_clone "$sm_path" "$name" "$url" "$reference" 
> > "$depth" || exit
> > +   module_clone "$sm_path" "$name" "$url" "$reference" 
> > "$depth" "$config_branch" || exit
> 
> In the simple case (update=checkout, no branch specified) with the
> new checkout branch stuff in module_clone() this code here ends up
> calling checkout twice.  First for master and then here later with
> the sha1.  This feels a little bit double.

There is no guarantee that the remote master and the exact sha1 point
at the same commit.  Ideally we'd just clone the exact sha1 in this
case.

> I would prefer if we skip the checkout in module_clone() if its not
> necessary.

When I tried to drop the '' case here:

> > @@ -306,7 +307,15 @@ module_clone()
> > echo "gitdir: $rel/$a" >"$sm_path/.git"
> >  
> > rel=$(echo $a | sed -e 's|[^/][^/]*|..|g')
> > -   (clear_local_git_env; cd "$sm_path" && GIT_WORK_TREE=. git config 
> > core.worktree "$rel/$b")
> > +   (
> > +   clear_local_git_env
> > +   cd "$sm_path" &&
> > +   GIT_WORK_TREE=. git config core.worktree "$rel/$b" &&
> > +   case "$branch" in
> > +   '') git checkout -f -q ;;
> > +   ?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
> > +   esac
> > +   ) || die "$(eval_gettext "Unable to setup cloned submodule 
> > '\$sm_path'")"
> >  }

I got test-suite errors that I didn't get to the bottom of.  However…

> How about we move the whole "what to checkout"-decision into one place
> instead of having it in update() and moving it from add() into
> module_clone() ?

…this sounds like a good idea to me.  However, it would be a more
intrusive change, and there may be conflicts with Francesco's proposed
attach/detach functionality.  I'll wait until we have a clearer idea
of where that is headed before I attempt a more complete
consolidation.

> > -   update_module= ;;
> > +   if test -n "$config_branch"; then
> > +   update_module="!git reset --hard -q"
> 
> If we get here the checkout has already been done. Shouldn't this
> rather specify a noop. I.E. like
> 
>   update_module="!true"

We are on a local branch at this point, but not neccessarily pointing
at the gitlinked sha1.  The reset here ensures that the new local
branch does indeed point at the gitlinked sha1.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


Re: [PATCH 1/2] git-submodule.sh: Support 'checkout' as a valid update command

2014-01-05 Thread W. Trevor King
On Sun, Jan 05, 2014 at 03:50:48AM +0100, Francesco Pretto wrote:
> + case "$update_module" in
> + '')
> + ;; # Unset update mode
> + checkout | rebase | merge | none)
> + ;; # Known update modes
> + !*)
> + ;; # Custom update command
> + *)
> + update_module=
> + echo >&2 "warning: invalid update mode for 
> submodule '$name'"
> + ;;
> + esac

I'd prefer `die "…"` to `echo >&2 "…"`.  It's hard to know if mapping
the user's preferred (unknown) update mechanism to 'checkout' is
serious or not.

This commit also makes me think that --rebase, --merge, and --checkout
should be replaced with a single --update={rebase|merge|checkout|!…}
option, but that's probably food for another commit (and a long
finger-breaking deprecation period).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


Re: [PATCH 2/2] Introduce git submodule attached update

2014-01-05 Thread Heiko Voigt
On Sun, Jan 05, 2014 at 03:50:49AM +0100, Francesco Pretto wrote:
> At the current state, the following use-case is not supported very
> well in git:
> - a maintainer adds a submodule, checking out a specific branch of
> the repository. He doesn't track the upstream submodule revision sha1;
> - a developer checkout the repository branch decided by the maintainer.
> Subsequent "merge" or "rebase" update operations don't detach the HEAD.

Could you please extend the description of your use-case so we can
understand your goal better?

The following questions directly pop into my mind:

 - What means the maintainer does not track the submodules sha1? Does
   that mean the superproject always refers to submodule commits using
   branches?
 - What happens if you want to go back to an earlier revision? Lets say
   a tagged release? How is ensured that you get the correct revision in
   the submodules?
 - In which situations does the developer or maintainer switch between
   your attached/detached mode?
 - What is the "repository branch" which is given to the developer by
   the maintainer used for? Who creates this branch and who merges into
   it?
 - What are these subsequent "merge" or "rebase" update operations? Do
   you mean everyone has submodule.name.update configured to merge or
   rebase?

Still puzzled.

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: [PATCH 1/2] git-submodule.sh: Support 'checkout' as a valid update command

2014-01-05 Thread Heiko Voigt
On Sun, Jan 05, 2014 at 03:50:48AM +0100, Francesco Pretto wrote:
> According to "Documentation/gitmodules.txt", 'checkout' is a valid
> 'submodule..update' command. Also "git-submodule.sh" refers to
> it and processes it correctly. Reflect commit 'ac1fbb' to support this
> syntax and also validates property values during 'update' command,
> issuing a warning if the value found is unknwon.

s/unknwon/unknown/

> ---
>  git-submodule.sh | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 2677f2e..1d041a7 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -622,7 +622,7 @@ cmd_init()
>  test -z "$(git config submodule."$name".update)"
>   then
>   case "$upd" in
> - rebase | merge | none)
> + checkout | rebase | merge | none)
>   ;; # known modes of updating
>   *)
>   echo >&2 "warning: unknown update mode '$upd' 
> suggested for submodule '$name'"
> @@ -805,6 +805,18 @@ cmd_update()
>   update_module=$update
>   else
>   update_module=$(git config submodule."$name".update)
> + case "$update_module" in
> + '')
> + ;; # Unset update mode
> + checkout | rebase | merge | none)
> + ;; # Known update modes
> + !*)
> + ;; # Custom update command
> + *)
> + update_module=
> + echo >&2 "warning: invalid update mode for 
> submodule '$name'"

How about additionally telling the user the current value that is wrong
like this:

echo >&2 "warning: invalid update mode '$update_module' for submodule 
'$name'"

?

But apart from those minor nits the patch looks good to me.

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: [PATCH 2/2] Introduce git submodule attached update

2014-01-05 Thread Francesco Pretto
(Hmmpth, forgot signoff...)

To whom it may interest, added some CC.

2014/1/5 Francesco Pretto :
> At the current state, the following use-case is not supported very
> well in git:
> - a maintainer adds a submodule, checking out a specific branch of
> the repository. He doesn't track the upstream submodule revision sha1;
> - a developer checkout the repository branch decided by the maintainer.
> Subsequent "merge" or "rebase" update operations don't detach the HEAD.
>
> To ease the above use-case this patch:
> - introduces a "submodule..attached" property that, when set
>   to "true", ensures that the "update" operation will result in
>   the HEAD attached to a branch;
> - introduces "--attach|--dettach" switches to the submodule "update"
>   command: they attach/detach the HEAD, overriding
>   "submodule..attached" property value;
> - introduces "--attached-update" switch to the "add" operation. It:
> * sets "submodule..attached" to true;
> * sets "submodule..ignore" to all.
>
> Using the '--attach' switch or operating in a repository with
> 'submodule..attached' set to 'true' during "update" will:
> - checkout a branch with an attached HEAD if the repository was just
> cloned;
> - perform a fast-forward only merge of changes if it's a 'checkout'
> update operation;
> - reattach the HEAD prior performing a 'merge', 'rebase' or '!command'
> update operation if the HEAD was found detached. Orphaned commits
> will also be merged back in the branch.
>
> '--attach' or 'submodule..attached' set to true also implies '--remote'.
>
> Using  the '--detach' switch or operating in a repository with
> 'submodule..attached' set to 'false' during "update" will:
> - checkout a detached HEAD if the repository was just cloned;
> - detach the HEAD prior performing a 'merge', 'rebase' or '!command'
> update operation if the HEAD was found attached.
>
> 'submodule..attached' works similarly to 'submodule..update'
> property: git copies the values found in ".gitmodules" in ".git/config" when
> performing an "init" command. "update" looks for values in ".git/config"
> only.
>
> '--attach' and '--detach' switches override an opposite behaviour
> of 'submodule..attached' properties.
>
> The patch is strongly additive and doesn't break any submodule specific
> test. It also adds some tests specific to the added feature.
> ---
>  Documentation/git-submodule.txt|  48 +--
>  Documentation/gitmodules.txt   |  10 +-
>  git-submodule.sh   | 154 +++--
>  t/t7410-submodule-attached-head.sh | 268 
> +
>  4 files changed, 457 insertions(+), 23 deletions(-)
>  create mode 100755 t/t7410-submodule-attached-head.sh
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index bfef8a0..b97eefb 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -10,13 +10,14 @@ SYNOPSIS
>  
>  [verse]
>  'git submodule' [--quiet] add [-b ] [-f|--force] [--name ]
> - [--reference ] [--depth ] [--]  
> []
> + [--reference ] [--attached-update] [--depth ]
> + [--]  []
>  'git submodule' [--quiet] status [--cached] [--recursive] [--] [...]
>  'git submodule' [--quiet] init [--] [...]
>  'git submodule' [--quiet] deinit [-f|--force] [--] ...
>  'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
> - [-f|--force] [--rebase] [--reference ] [--depth 
> ]
> - [--merge] [--recursive] [--] [...]
> + [-f|--force] [--rebase] [--reference ] [--attach | 
> --detach]
> + [--depth ] [--merge] [--recursive] [--] [...]
>  'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) 
> ]
>   [commit] [--] [...]
>  'git submodule' [--quiet] foreach [--recursive] 
> @@ -107,6 +108,9 @@ is the superproject and submodule repositories will be 
> kept
>  together in the same relative location, and only the
>  superproject's URL needs to be provided: git-submodule will correctly
>  locate the submodule using the relative URL in .gitmodules.
> ++
> +If `--attached-update` is specified, the property `submodule..attached`
> +will be set to `true` and `submodule..ignore` will be set to `all`.
>
>  status::
> Show the status of the submodules. This will print the SHA-1 of the
> @@ -156,12 +160,15 @@ it contains local modifications.
>  update::
> Update the registered submodules, i.e. clone missing submodules and
> checkout the commit specified in the index of the containing 
> repository.
> -   This will make the submodules HEAD be detached unless `--rebase` or
> -   `--merge` is specified or the key `submodule.$name.update` is set to
> -   `rebase`, `merge` or `none`. `none` can be overridden by specifying
> -   `--checkout`. Setting the key `submodule.$name.update` to `!command`
> -   will cause `command` to be run. `command` can be any arbitrary shell
> -

Re: [RFC v2] submodule: Respect requested branch on all clones

2014-01-05 Thread Heiko Voigt
On Sun, Jan 05, 2014 at 08:17:00AM -0800, W. Trevor King wrote:
> From: "W. Trevor King" 
> 
> The previous code only checked out the requested branch in cmd_add.
> This commit moves the branch-checkout logic into module_clone, where
> it can be shared by cmd_add and cmd_update.  I also update the initial
> checkout command to use 'rebase' to preserve branches setup during
> module_clone.
> 
> Signed-off-by: W. Trevor King 
> ---
> Changes since v1:
> * Fix a 'reqested' -> 'requested' typo in the subject/summary.
> * Restore a post-clone 'git checkout -f -q' for the empty-branch case
>   in module_clone().
> * Distinguish between $branch (which defaults to 'master') and a new
>   $config_branch (which defaults to empty) in cmd_update
> 
> After these fixes, all the existing submodule tests pass.  If we want
> to merge this, we'd still want new tests that demonstrate the new
> functionality.

I think this patch is going in the right direction. Making add() and
update() do the same is the right thing to do.

I would still like a complete description of Francesco's use case
though. Francesco: Could you give us a short description about what
exactly is your use-case? And please ignore all technical details how we
are going to implement this. I would like to know how, in an ideal
world, you would expect git to behave. Do you really only care about

git submodule add

and the *initial*

git submodule update

?

What happens if a developer already has the submodule and wants to work
on a feature?

> On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote:
> > If I understand it correctly, looking at your intervention in
> > module_clone and cmd_update, when "submodule..branch" is set
> > during "update" the resulting first clone will always be a branch
> > checkout (cause $branch is filled with "branch" property).
> 
> That's correct.
> 
> > I believe this will break a lot of tests,
> 
> Thanks for prompting me to run the tests.  This v2 now passes all of
> the current submodule tests, and the functionality actually matches my
> earlier descriptions of it ;).
> 
> > as the the documentation says that in this configuration the HEAD
> > should be detached.
> 
> The current Documentation/git-submodule.txt has:
> 
>   update::
> Update the registered submodules, i.e. clone missing submodules
> and checkout the commit specified in the index of the containing
> repository.  This will make the submodules HEAD be detached unless
> `--rebase` or `--merge` is specified or the key
> `submodule.$name.update` is set to `rebase`, `merge` or `none`.
> 
> It's not clear if this refers to the initial-clone update, future
> post-clone updates, or both.  Ideally, the behavior should be the
> same, but in the initial-clone case we don't have an existing
> checked-out branch to work with.

I do not think that its actually important to end up with a detached
HEAD. The documentation just states it because in most cases there is no
other option. But I do not think anything will break if a branch points
to the exact sha1 we would checkout and we checkout the branch instead.

> > Also it could break some users that rely on the current behavior.
> 
> The current code always has a detached HEAD after an initial-clone
> update, regardless of submodule..update, which doesn't match
> those docs either.  Adding a check to only checkout
> submodule..branch if submodule..update was 'rebase',
> 'merge', or 'none' would be easy, but I don't think that makes much
> sense.  I can't see any reason for folks who specify
> submodule..branch to prefer a detached HEAD over a local branch
> matching the remote branch's name.  If they prefer checkout updates,
> the first such update will return them to a detached HEAD.  If they
> prefer merge/rebase updates, future updates will keep them on the same
> branch.  All my commit does is setup that initial branch for folks who
> get the submodule via 'update', in the same way it's currently setup
> for folks who get the submodule via 'add'.

I like your goal of putting this logic into one place. But a few things
still could be improved IMO.

>  git-submodule.sh | 34 --
>  1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 2979197..167d4fa 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -253,6 +253,7 @@ module_clone()
>   url=$3
>   reference="$4"
>   depth="$5"
> + branch="$6"
>   quiet=
>   if test -n "$GIT_QUIET"
>   then
> @@ -306,7 +307,15 @@ module_clone()
>   echo "gitdir: $rel/$a" >"$sm_path/.git"
>  
>   rel=$(echo $a | sed -e 's|[^/][^/]*|..|g')
> - (clear_local_git_env; cd "$sm_path" && GIT_WORK_TREE=. git config 
> core.worktree "$rel/$b")
> + (
> + clear_local_git_env
> + cd "$sm_path" &&
> + GIT_WORK_TREE=. git config core.worktree "$rel/$b" &&
> + case "$branc

[RFC v2] submodule: Respect requested branch on all clones

2014-01-05 Thread W. Trevor King
From: "W. Trevor King" 

The previous code only checked out the requested branch in cmd_add.
This commit moves the branch-checkout logic into module_clone, where
it can be shared by cmd_add and cmd_update.  I also update the initial
checkout command to use 'rebase' to preserve branches setup during
module_clone.

Signed-off-by: W. Trevor King 
---
Changes since v1:
* Fix a 'reqested' -> 'requested' typo in the subject/summary.
* Restore a post-clone 'git checkout -f -q' for the empty-branch case
  in module_clone().
* Distinguish between $branch (which defaults to 'master') and a new
  $config_branch (which defaults to empty) in cmd_update

After these fixes, all the existing submodule tests pass.  If we want
to merge this, we'd still want new tests that demonstrate the new
functionality.

On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote:
> If I understand it correctly, looking at your intervention in
> module_clone and cmd_update, when "submodule..branch" is set
> during "update" the resulting first clone will always be a branch
> checkout (cause $branch is filled with "branch" property).

That's correct.

> I believe this will break a lot of tests,

Thanks for prompting me to run the tests.  This v2 now passes all of
the current submodule tests, and the functionality actually matches my
earlier descriptions of it ;).

> as the the documentation says that in this configuration the HEAD
> should be detached.

The current Documentation/git-submodule.txt has:

  update::
Update the registered submodules, i.e. clone missing submodules
and checkout the commit specified in the index of the containing
repository.  This will make the submodules HEAD be detached unless
`--rebase` or `--merge` is specified or the key
`submodule.$name.update` is set to `rebase`, `merge` or `none`.

It's not clear if this refers to the initial-clone update, future
post-clone updates, or both.  Ideally, the behavior should be the
same, but in the initial-clone case we don't have an existing
checked-out branch to work with.

> Also it could break some users that rely on the current behavior.

The current code always has a detached HEAD after an initial-clone
update, regardless of submodule..update, which doesn't match
those docs either.  Adding a check to only checkout
submodule..branch if submodule..update was 'rebase',
'merge', or 'none' would be easy, but I don't think that makes much
sense.  I can't see any reason for folks who specify
submodule..branch to prefer a detached HEAD over a local branch
matching the remote branch's name.  If they prefer checkout updates,
the first such update will return them to a detached HEAD.  If they
prefer merge/rebase updates, future updates will keep them on the same
branch.  All my commit does is setup that initial branch for folks who
get the submodule via 'update', in the same way it's currently setup
for folks who get the submodule via 'add'.

Cheers,
Trevor

 git-submodule.sh | 34 --
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197..167d4fa 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -253,6 +253,7 @@ module_clone()
url=$3
reference="$4"
depth="$5"
+   branch="$6"
quiet=
if test -n "$GIT_QUIET"
then
@@ -306,7 +307,15 @@ module_clone()
echo "gitdir: $rel/$a" >"$sm_path/.git"
 
rel=$(echo $a | sed -e 's|[^/][^/]*|..|g')
-   (clear_local_git_env; cd "$sm_path" && GIT_WORK_TREE=. git config 
core.worktree "$rel/$b")
+   (
+   clear_local_git_env
+   cd "$sm_path" &&
+   GIT_WORK_TREE=. git config core.worktree "$rel/$b" &&
+   case "$branch" in
+   '') git checkout -f -q ;;
+   ?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
+   esac
+   ) || die "$(eval_gettext "Unable to setup cloned submodule 
'\$sm_path'")"
 }
 
 isnumber()
@@ -469,16 +478,7 @@ Use -f if you really want to add it." >&2
echo "$(eval_gettext "Reactivating local git 
directory for submodule '\$sm_name'.")"
fi
fi
-   module_clone "$sm_path" "$sm_name" "$realrepo" "$reference" 
"$depth" || exit
-   (
-   clear_local_git_env
-   cd "$sm_path" &&
-   # ash fails to wordsplit ${branch:+-b "$branch"...}
-   case "$branch" in
-   '') git checkout -f -q ;;
-   ?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
-   esac
-   ) || die "$(eval_gettext "Unable to checkout submodule 
'\$sm_path'")"
+   module_clone "$sm_path" "$sm_name" "$realrepo" "$reference" 
"$depth" "$branch" || exit
fi
git config submodule."$sm_name".url "$realrepo"
 
@@ -787,7 +787

Re: [PATCH v2 3/4] Speed up is_git_command() by checking early for internal commands

2014-01-05 Thread Sebastian Schuberth
On Fri, Jan 3, 2014 at 5:49 PM, Kent R. Spillner  wrote:

>> Since 2dce956 is_git_command() is a bit slow as it does file I/O in the
>> call to list_commands_in_dir(). Avoid the file I/O by adding an early
>> check for internal commands.
>
> Considering the purpose of the series is it better to say "builtin" instead 
> of "internal" in the commit message?

True, I'll fix this in a re-rool.

-- 
Sebastian Schuberth
--
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 v4 2/4] completion: introduce __gitcomp_nl_append ()

2014-01-05 Thread Ramkumar Ramachandra
There are situations where multiple classes of completions possible. For
example

  branch.

should try to complete

  branch.master.
  branch.autosetupmerge
  branch.autosetuprebase

The first candidate has the suffix ".", and the second/ third candidates
have the suffix " ". To facilitate completions of this kind, create a
variation of __gitcomp_nl () that appends to the existing list of
completion candidates, COMPREPLY.

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 22 ++
 contrib/completion/git-completion.zsh  |  8 
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 51c2dd4..20febff 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -178,9 +178,9 @@ _get_comp_words_by_ref ()
 }
 fi
 
-__gitcompadd ()
+__gitcompappend ()
 {
-   local i=0
+   local i=${#COMPREPLY[@]}
for x in $1; do
if [[ "$x" == "$3"* ]]; then
COMPREPLY[i++]="$2$x$4"
@@ -188,6 +188,12 @@ __gitcompadd ()
done
 }
 
+__gitcompadd ()
+{
+   COMPREPLY=()
+   __gitcompappend "$@"
+}
+
 # Generates completion reply, appending a space to possible completion words,
 # if necessary.
 # It accepts 1 to 4 arguments:
@@ -218,6 +224,14 @@ __gitcomp ()
esac
 }
 
+# Variation of __gitcomp_nl () that appends to the existing list of
+# completion candidates, COMPREPLY.
+__gitcomp_nl_append ()
+{
+   local IFS=$'\n'
+   __gitcompappend "$1" "${2-}" "${3-$cur}" "${4- }"
+}
+
 # Generates completion reply from newline-separated possible completion words
 # by appending a space to all of them.
 # It accepts 1 to 4 arguments:
@@ -229,8 +243,8 @@ __gitcomp ()
 #appended.
 __gitcomp_nl ()
 {
-   local IFS=$'\n'
-   __gitcompadd "$1" "${2-}" "${3-$cur}" "${4- }"
+   COMPREPLY=()
+   __gitcomp_nl_append "$@"
 }
 
 # Generates completion reply with compgen from newline-separated possible
diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index 6fca145..6b77968 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -76,6 +76,14 @@ __gitcomp_nl ()
compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
 }
 
+__gitcomp_nl_append ()
+{
+   emulate -L zsh
+
+   local IFS=$'\n'
+   compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
+}
+
 __gitcomp_file ()
 {
emulate -L zsh
-- 
1.8.5.2.227.g53f3478

--
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 v4 1/4] zsh completion: find matching custom bash completion

2014-01-05 Thread Ramkumar Ramachandra
If zsh completion is being read from a location that is different from
system-wide default, it is likely that the user is trying to use a
custom version, perhaps closer to the bleeding edge, installed in her
own directory. We will more likely to find the matching bash completion
script in the same directory than in those system default places.

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.zsh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index fac5e71..6fca145 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -30,10 +30,10 @@ if [ -z "$script" ]; then
local -a locations
local e
locations=(
+   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
'/etc/bash_completion.d/git' # fedora, old debian
'/usr/share/bash-completion/completions/git' # arch, ubuntu, 
new debian
'/usr/share/bash-completion/git' # gentoo
-   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
)
for e in $locations; do
test -f $e && script="$e" && break
-- 
1.8.5.2.227.g53f3478

--
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 v4 4/4] completion: fix remote.pushdefault

2014-01-05 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config remote.push

'pushdefault' doesn't come up. This is because "$cur" is matched with
"remote.*" and a list of remotes are completed. Add 'pushdefault' as a
candidate for completion too, using __gitcomp_nl_append ().

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index a57bcbe..4fe5ce3 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1884,6 +1884,7 @@ _git_config ()
remote.*)
local pfx="${cur%.*}." cur_="${cur#*.}"
__gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "."
+   __gitcomp_nl_append "pushdefault" "$pfx" "$cur_"
return
;;
url.*.*)
-- 
1.8.5.2.227.g53f3478

--
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 v4 0/4] Re-spin rr/completion-branch-config

2014-01-05 Thread Ramkumar Ramachandra
Hi Junio et al,

Most significantly, [2/4] no longer duplicates __gitcompadd () in
__gitcomp_nl_append (). Other than that, the commit messages for the
other patches are improved.

Thanks.

Ramkumar Ramachandra (4):
  zsh completion: find matching custom bash completion
  completion: introduce __gitcomp_nl_append ()
  completion: fix branch.autosetup(merge|rebase)
  completion: fix remote.pushdefault

 contrib/completion/git-completion.bash | 24 
 contrib/completion/git-completion.zsh  | 10 +-
 2 files changed, 29 insertions(+), 5 deletions(-)

-- 
1.8.5.2.227.g53f3478

--
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 v4 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-05 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config branch.auto

'autosetupmerge' and 'autosetuprebase' don't come up. This is because
"$cur" is matched with "branch.*" and a list of branches are
completed. Add 'autosetupmerge', 'autosetuprebase' as candidates for
completion too, using __gitcomp_nl_append ().

Signed-off-by: Ramkumar Ramachandra 
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 20febff..a57bcbe 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1841,6 +1841,7 @@ _git_config ()
branch.*)
local pfx="${cur%.*}." cur_="${cur#*.}"
__gitcomp_nl "$(__git_heads)" "$pfx" "$cur_" "."
+   __gitcomp_nl_append $'autosetupmerge\nautosetuprebase\n' "$pfx" 
"$cur_"
return
;;
guitool.*.*)
-- 
1.8.5.2.227.g53f3478

--
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 v2 1/4] completion: prioritize ./git-completion.bash

2014-01-05 Thread Ramkumar Ramachandra
brian m. carlson wrote:
> I'm not clear on this change.  It looks like this loads
> git-completion.bash from the same directory as git-completion.zsh.  Is
> this correct?

Yes, and that's what I meant to convey with the "./". Junio's message
is clearer though, so I'll use that.

Thanks.
--
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 v3 2/4] completion: introduce __gitcomp_nl_append ()

2014-01-05 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> Is it because going this route and doing it at such a low level
> would make zsh completion (which I have no clue about ;-)
> unnecessarily complex?

The zsh completion only cares to override __gitcomp_nl () and
__gitcomp_nl_append (), without bothering to re-implement the
lower-level functions; so it's no problem at all. I wrote mine out by
thinking of a non-intrusive direct translation, while your version
focuses on eliminating duplication. I don't have a strong preference
either way, so I will resubmit with your version.
--
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


Aw: Re: [PATCH] Improve user-manual html and pdf formatting

2014-01-05 Thread Thomas Ackermann
 
This originally was an UTF8-BOM in user-manual.txt and notepad++ was so clever
not to show it in the patch-file :-| Pasting this into my webmail then produced 
complete rubbish which I didn't noticed ...

- Original Nachricht 
Von: Jonathan Nieder 
An:  Thomas Ackermann 
Datum:   04.01.2014 22:18
Betreff: Re: [PATCH] Improve user-manual html and pdf formatting

> Hi,
> 
> Thomas Ackermann wrote:
> 
> > --- a/Documentation/user-manual.txt
> > +++ b/Documentation/user-manual.txt
> > @@ -1,5 +1,5 @@
> > -Git User Manual
> > +Git User Manual
> 
> Why?
> 
> Puzzled,
> Jonathan
> 

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