Re: [PATCH/RFC] Introduce git submodule add|update --attach

2014-01-13 Thread Junio C Hamano
Francesco Pretto cez...@gmail.com writes:

 Thanks for the comments, my replies below. Before, a couple of general
 questions:
 - I'm also writing some tests, should I commit them together with the
 feature patch?
 - to determine the attached/detached state I did this:

 head_detached=
 if test $(rev-parse --abbrev-ref HEAD) = HEAD
 then
 head_detached=true
 fi

Use git symbolic-ref HEAD to read off of 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


Re: [PATCH/RFC] Introduce git submodule add|update --attach

2014-01-03 Thread Francesco Pretto
2014/1/3 Francesco Pretto cez...@gmail.com:
 Concluding, my point is that at the current state submodules in git
 seem to be flawed because of the inconsistent HEAD state between add
 and update users. With my patch applied the attached HEAD behavior
 would be fully supported. At some point git submodule add (without
 the --attached switch) could be also modified to produce a detached
 HEAD by default, removing any remaining inconsistency.

Junio, please let me amend this affirmation as it's inaccurate.
According to the current *upstream* features this should the supported
use case for submodules:
- there's a maintainer add user that adds the submodule. He will
need to track the upstream submodule revision sha1, so he will clone
the repository with an *attached* HEAD;
- there's a developer update use that just clone the submodule
repository and track the sha1 decided by the maintainer add user. He
won't need to track upstream submodule revision sha1 so cloning the
repository with a *detached* HEAD. Subsequent merge or rebase
update operations will keep the HEAD detached.

We should *not* modify/break this default workflow in any way.

The current documentation seems to be misleading when saying that
This [the update command] will make the submodules HEAD be detached
**unless** --rebase or --merge is specified, as it seems to imply
that the update operation will result in a repository with the HEAD
attached. The repository will be indeed updated merging changes, but
the HEAD will stay detached.

Now, the use case I would like git to support is this:
- there's a maintainer add user that adds the submodule. He won't
track the upstream submodule revision sha1, so
submodule.module.ignore will be set to all. He will still clone
the repository with an *attached* HEAD;
- there's a developer update user. He will clone the submodule
repository with an *attached* HEAD. Subsequent merge or rebase
update operations will keep the HEAD attached.

To fully support this workflow in my patch I want to:
- introduce a submodule.module.attach property so update users
will clone the submodule repository with an attached HEAD;
- introduce --attach|--dettach switches to update commmant to
eventually override submodule.module.attach property value in the
command line;
- introduce --attach (but better rename it to --keep-attached or
something else) switch to the add operation. This --keep-attached
will:
* set submodule.module.attach to true;
* set submodule.module.ignore to all.

Being the workflow complementary, and the behavior fully optional, I
think the proposed feature should be solid. Give me some time to
produce an improved patch and let see if I can convince you.

Regards,
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: [PATCH/RFC] Introduce git submodule add|update --attach

2014-01-02 Thread Francesco Pretto
Thanks for the comments, my replies below. Before, a couple of general
questions:
- I'm also writing some tests, should I commit them together with the
feature patch?
- to determine the attached/detached state I did this:

head_detached=
if test $(rev-parse --abbrev-ref HEAD) = HEAD
then
head_detached=true
fi

Is this correct?

2013/12/31 Phil Hord phil.h...@gmail.com

 On Sun, Dec 29, 2013 at 8:49 PM, Francesco Pretto cez...@gmail.com wrote:
  [...]
 
  When updating, using the '--attach' switch or operating in a repository with
  'submodule.name.attach' set to 'true' 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,
  keeping the HEAD attached;
  - reattach the HEAD prior performing a 'merge', 'rebase' or '!command' 
  update
  operation if the HEAD was found detached.

 I need to understand this reattach the HEAD case better. Can you
 give some examples of the expected behavior when merge, rebase or
 !command is encountered?


Thanks for pointing this out, actually my patch was a bit lacking at
this. Reattaching the HEAD prior to merge, rebase or !command would
have caused just a *silent* git checkout branch, possibly leaving
orphaned commits forgotten.

My plans for the patch are now to implement this safer and, IMO,
intuitive behavior: let set say we have a submodule mod with the
HEAD detached at orphaned commit orphaned-sha1. Let say
origin/branch is at commit origin-sha1. Let say I set
submodule.mod.attach to true or I run git submodule update with
the --attach switch. The expected behavior for submodule mod would
be (pseudocode):

git checkout branch
if merge and head_detached then
git merge orphaned-sha1
case:
merge:
git merge origin-sha1
rebase:
git rebase origin-sha1
!command:
command origin-sha1
if rebase and head_detached then
   git merge orphaned-sha1

So, in both merge|rebase cases we merge back orphaned commits with a
git merge, but the effect will be a merge or a rebase depending of
the ordering of the main update operation. We can't assume a merge
or rebase operation in the case of !command so we let the
responsibility of merging back orphaned commits to the user.

Sounds good?


  +
  +--attach::
  +   This option is only valid for the add and update commands. Cause the

 Grammar: 'Causes the result'


Ok.


  +   result of an add or update operation to be an attached HEAD. In the
  +   update command , if `submodule.name.branch` is not set, it will

 typo: space before comma.


Ok.


 Also, the pronoun it here is unclear to me.  Does this convey the
 correct meaning?

In the update operation, the branch named by 'submodule.name.branch' is
checked out as the new HEAD of the submodule repository. If
'submodule.name.branch' is not set, the 'master' branch is
 checked out as the
new HEAD of the submodule.


Sounds good to me.


  +   default to `master`. Note: for the update command `--attach` also
  +   implies `--remote`.
  +
  +--detach::
  +   This option is only valid for the update command. Cause the result

  Grammar: 'Causes the result'


Ok.


  +   of the update operation to be forcedly a detached HEAD.

 Forcedly is a bit strong, maybe, slightly misplaced, and not a word,
 besides.   How's this, instead:

Forces the result of the update operation to be a detached HEAD in
 the submodule.


Sounds good to me.


   submodule.name.update::
  Defines what to do when the submodule is updated by the 
  superproject.
  -   If 'checkout' (the default), the new commit specified in the
  -   superproject will be checked out in the submodule on a detached 
  HEAD.
  +   If 'checkout' (the default), the new commit (or the branch, when 
  using
  +   the '--attach' switch or the 'submodule.name.attach' property is 
  set
  +   to 'true' during an update operation) specified in the superproject 
  will
  +   be checked out in the submodule.

 IMHO, this wording is overcomplicated by this change.  How about:

If 'checkout' (the default), the new commit specified in the 
 superproject
(or branch, with '--attach') will be checked out in the submodule.


Sounds good to me.


  If 'rebase', the current branch of the submodule will be rebased 
  onto
  the commit specified in the superproject. If 'merge', the commit
  specified in the superproject will be merged into the current branch

 Does the 'merge', 'rebase' and '!command' description need to be
 updated, too?  Here and above it seems to still suggest the old
 behavior is kept when --attach is used.


You are right, I'll improve those. Also I think the documentation was
a bit innacurate because, with the current git features state, it's
possible merge changes keeping the HEAD detached, and that's what
happen.



  @@ -54,6 +56,11 @@ submodule.name.branch::
  If 

Re: [PATCH/RFC] Introduce git submodule add|update --attach

2014-01-02 Thread Junio C Hamano
Francesco Pretto cez...@gmail.com writes:

 by default git submodule performs its add or update operations on a detached
 HEAD. This works well when using an existing full-fledged/indipendent project 
 as
 the submodule, as there's less frequent need to update it or commit back
 changes. When the submodule is actually a large portion of shareable code
 between  different projects, and the superproject needs to track very closely
 the evolution of the submodule (or the other way around), I feel more 
 confortable
 to reattach the HEAD of the submodule with an existing branch.

I may be missing some fundamental assumption in your mind when you
did this change, but in a workflow where somebody wants submodule
checkout to be on branches (as opposed to detached), wouldn't it
make more sense not to detach in the first place, rather than
introducing yet another option to re-attach?  The documentation of
submodule update seems to say that its merge and rebase modes
do not detach in the first place (and it alludes to --checkout but
it is unclear what it does purely from the documentation, as git
submodule --help does not even list it as one of the options).

And if there is a good reason why detaching to update and then
(perhaps after verifying the result or cleaning it up?  I dunno what
the expected use case is, so I am purely guessing) attaching the
result to a specific branch in separate steps, does it make sense to
give --attach option to update in the first place?  That makes
the whole thing into a single step, not giving the user a chance to
do anything in between, which I am guessing is the whole point of
your not using the existing do not detach, work on a branch modes.

Puzzled...
--
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/RFC] Introduce git submodule add|update --attach

2014-01-02 Thread Francesco Pretto
2014/1/2 Junio C Hamano gits...@pobox.com:
 Francesco Pretto cez...@gmail.com writes:

 by default git submodule performs its add or update operations on a 
 detached
 HEAD. This works well when using an existing full-fledged/indipendent 
 project as
 the submodule, as there's less frequent need to update it or commit back
 changes. When the submodule is actually a large portion of shareable code
 between  different projects, and the superproject needs to track very closely
 the evolution of the submodule (or the other way around), I feel more 
 confortable
 to reattach the HEAD of the submodule with an existing branch.

 I may be missing some fundamental assumption in your mind when you
 did this change, but in a workflow where somebody wants submodule
 checkout to be on branches (as opposed to detached), wouldn't it
 make more sense not to detach in the first place, rather than
 introducing yet another option to re-attach?  The documentation of
 submodule update seems to say that its merge and rebase modes
 do not detach in the first place (and it alludes to --checkout but
 it is unclear what it does purely from the documentation, as git
 submodule --help does not even list it as one of the options).


Thanks for commenting: because of more checking I just spotted some
unuseful code in my patch in cmd_add() function. In short: it seems to
me git submodule update doesn't allow to work with attached an HEAD
(unless it is *manually* attached, as I regularly do). My feeling is
the documentation of merge, rebase update commands is inaccurate
and doesn't really reflect what happen when adding a submodule with
add or cloning it for the first time with update. You can look at
the following conditionals in the current code in git-submodule.sh:

## cmd_add()
case $branch in
'') git checkout -f -q ;;
?*) git checkout -f -q -B $branch origin/$branch ;;
esac

## cmd_update()
# Is this something we just cloned?
case ;$cloned_modules; in
*;$name;*)
# then there is no local change to integrate
update_module= ;;
esac

This means that the add command will always checkout an *attached*
HEAD but at the first clone of a different user the checkout will
resolve in git checkout sha1, always producing a *detached* HEAD
and resulting in inconsistent HEAD state between who added the
submodule with add and who cloned it with update. Subsequent
merge or rebase operations won't change this fact, the HEAD will
remain detached. The following test case confirms this, unless I did
something wrong:

-
parentdir=$(pwd -P)
submodurl1=$(pwd -P)/repo1
submodurl2=$(pwd -P)/repo2
repourl=$(pwd -P)/repo

# Create repo to be added with submodules
cd $parentdir
mkdir repo
cd repo
git init
git config receive.denyCurrentBranch ignore
echo a a
git add a
git commit -m repo commit 1

# Create repo repo1 to be used as submodule
cd $parentdir
mkdir repo1
cd repo1
git init
git config receive.denyCurrentBranch ignore
echo a a
git add a
git commit -m repo1 commit 1

# Create repo repo2 to be used as submodule
cd $parentdir
mkdir repo2
cd repo2
git init
git config receive.denyCurrentBranch ignore
echo a a
git add a
git commit -m repo2 commit 1

# Clone repo to test git submodule update
cd $parentdir
git clone $repourl repoclone

#
## Adding submodule with update rebase, not specifying a branch
#

cd $parentdir/repo
git submodule add $submodurl1 submod1
git config -f .gitmodules submodule.submod1.ignore all
git config -f .gitmodules submodule.submod1.update rebase
git commit -m Added submodule
## repo/submod1 has an attached HEAD ##

cd $parentdir/repoclone
git pull
git submodule init
git submodule update
## repoclone/submod1 has a detached HEAD -- note the inconsistency ##

#
## Adding submodule with update rebase, specifying a branch
#
cd $parentdir/repo
git submodule add --branch master $submodurl2 submod2
git config -f .gitmodules submodule.submod2.ignore all
git config -f .gitmodules submodule.submod2.update rebase
git add .
git commit -m Added submodule
## repo/submod2 has a attached HEAD ##

cd $parentdir/repoclone
git pull
git submodule init
git submodule update --remote
## repoclone/submod2 has a detached HEAD -- note the inconsistency ##

#
## Adding something to submod2 and test update rebase on repoclone
#

cd $parentdir/repo1
echo b b
git add b
git commit -m repo1 commit 2

cd $parentdir/repoclone
git submodule update --remote
## repoclone/submod1 has still a detached HEAD ##
-


 And if there is a good reason why detaching to update and then
 (perhaps after verifying the result or cleaning it up?  I dunno what
 the expected use case is, so I am purely guessing) attaching the
 result to a specific branch in separate steps, does it make sense to
 give --attach option to update in the first place?  That makes
 the whole thing into a single step, not giving the user a 

Re: [PATCH/RFC] Introduce git submodule add|update --attach

2013-12-31 Thread Phil Hord
On Sun, Dec 29, 2013 at 8:49 PM, Francesco Pretto cez...@gmail.com wrote:

 by default git submodule performs its add or update operations on a detached
 HEAD. This works well when using an existing full-fledged/indipendent project 
 as
 the submodule, as there's less frequent need to update it or commit back
 changes. When the submodule is actually a large portion of shareable code
 between  different projects, and the superproject needs to track very closely
 the evolution of the submodule (or the other way around), I feel more 
 confortable
 to reattach the HEAD of the submodule with an existing branch. This can be as
 simple as having a superproject project1 in branch master with a submodule
 common attached to the branch master-project1 or, in a more development
 workflow, project1 in branch featureA with the same submodule common
 attached to a similarly named branch featureA. Doing this in git requires me
 the following:

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

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

 While the burden for the repository maitainer/administrator is acceptable, in
 the developer point of view there are two problems:
 1) Checking out an attached HEAD of a specified branch as when using 
 --remote
 is not really simple as it could be and could require lauching of scrips or
 reading some repository specific documentation. Also in Windows platform the
 syntax for inline shell evaluation of commands is less known between users;
 2) There's no way to store a similar default behaviour in the repository 
 except
 by using scripts. Also recently submodule.modulename.update custom !commands
 in no more supported when stored in .gitmodules [1].

 The attached patch tries to solve these problems by introducing an --attach
 switch to the add and update submodule commands and a --detach switch 
 just
 for the update command. It also add the support for an 
 'submodule.name.attach'
 property when updating. Using the --attach switch when adding a submodule 
 does:
 - create the submodule checking out an attached HEAD;
 - set the 'submodule.name.attach' property to 'true';
 - set the 'submodule.name.ignore' property to 'all' (this is useful as
 attaching to the branch doesn't require tracking of revision sha1).

 The rationale of setting 'attach' and 'ignore' properties when adding a
 submodule with the --attach switch is to give a convenient default 
 behaviour.
 No other properties are set: the repository responsible will still be required
 to configure a different 'submodule.name.update' behaviour separetely, if he
 wants that.

 When updating, using the '--attach' switch or operating in a repository with
 'submodule.name.attach' set to 'true' 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,
 keeping the HEAD attached;
 - reattach the HEAD prior performing a 'merge', 'rebase' or '!command' update
 operation if the HEAD was found detached.

I need to understand this reattach the HEAD case better. Can you
give some examples of the expected behavior when merge, rebase or
!command is encountered?


 '--attach' or 'submodule.name.attach' set to true also implies '--remote', 
 as
 it's needed the origin HEAD sha1 to verify the current HEAD state.

 A '--detach' switch is also available. Using the '--detach' switch or
 operating in a repository with 'submodule.name.attach' set to 'false' during
 update will:
 - checkout a detached HEAD if the repository was just cloned (same behaviour 
 as
 before);
 - detach the HEAD prior performing a 'merge', 'rebase' or '!command' update
 operation if the HEAD was found attached.

 'submodule.name.attach' works the same way as 'submodule.name.update'
 property: git copies the values found in .gitmodules in .git/config when
 performing an init command. update looks for the values in .git/config
 only.

 '--attach' and '--detach' switches override an opposite behaviour of 
 'submodule.name.attach'
 properties.

 The patch is small (touches only git-submodule.sh) and 100% additive with
 respect to currently documented behaviour: when using add and update
 commands without the introduced switches and properties, git shall operate as
 before. As a bonus (but this was done to ease conditionals and keep the code
 clean) it also clarifies and validates the content of 
 'submodule.name.update'
 during 'update' command, warning the user if it's not one of the supported
 values 'checkout', 'merge', 'rebase' and 'none'. Please note that 'checkout'
 update command was documented in upstream Documentation/gitmodules.txt [2] 
 as
 a valid 

[PATCH/RFC] Introduce git submodule add|update --attach

2013-12-29 Thread Francesco Pretto
Hello everybody,

by default git submodule performs its add or update operations on a detached
HEAD. This works well when using an existing full-fledged/indipendent project as
the submodule, as there's less frequent need to update it or commit back
changes. When the submodule is actually a large portion of shareable code
between  different projects, and the superproject needs to track very closely
the evolution of the submodule (or the other way around), I feel more 
confortable
to reattach the HEAD of the submodule with an existing branch. This can be as
simple as having a superproject project1 in branch master with a submodule
common attached to the branch master-project1 or, in a more development
workflow, project1 in branch featureA with the same submodule common
attached to a similarly named branch featureA. Doing this in git requires me
the following:

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

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

While the burden for the repository maitainer/administrator is acceptable, in
the developer point of view there are two problems:
1) Checking out an attached HEAD of a specified branch as when using --remote
is not really simple as it could be and could require lauching of scrips or
reading some repository specific documentation. Also in Windows platform the
syntax for inline shell evaluation of commands is less known between users;
2) There's no way to store a similar default behaviour in the repository except
by using scripts. Also recently submodule.modulename.update custom !commands
in no more supported when stored in .gitmodules [1].

The attached patch tries to solve these problems by introducing an --attach
switch to the add and update submodule commands and a --detach switch just
for the update command. It also add the support for an 
'submodule.name.attach'
property when updating. Using the --attach switch when adding a submodule 
does:
- create the submodule checking out an attached HEAD;
- set the 'submodule.name.attach' property to 'true';
- set the 'submodule.name.ignore' property to 'all' (this is useful as
attaching to the branch doesn't require tracking of revision sha1).

The rationale of setting 'attach' and 'ignore' properties when adding a
submodule with the --attach switch is to give a convenient default behaviour.
No other properties are set: the repository responsible will still be required
to configure a different 'submodule.name.update' behaviour separetely, if he
wants that.

When updating, using the '--attach' switch or operating in a repository with
'submodule.name.attach' set to 'true' 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,
keeping the HEAD attached;
- reattach the HEAD prior performing a 'merge', 'rebase' or '!command' update
operation if the HEAD was found detached.

'--attach' or 'submodule.name.attach' set to true also implies '--remote', as
it's needed the origin HEAD sha1 to verify the current HEAD state.

A '--detach' switch is also available. Using the '--detach' switch or
operating in a repository with 'submodule.name.attach' set to 'false' during
update will:
- checkout a detached HEAD if the repository was just cloned (same behaviour as
before);
- detach the HEAD prior performing a 'merge', 'rebase' or '!command' update
operation if the HEAD was found attached.

'submodule.name.attach' works the same way as 'submodule.name.update'
property: git copies the values found in .gitmodules in .git/config when
performing an init command. update looks for the values in .git/config
only.

'--attach' and '--detach' switches override an opposite behaviour of 
'submodule.name.attach'
properties.

The patch is small (touches only git-submodule.sh) and 100% additive with
respect to currently documented behaviour: when using add and update
commands without the introduced switches and properties, git shall operate as
before. As a bonus (but this was done to ease conditionals and keep the code
clean) it also clarifies and validates the content of 'submodule.name.update'
during 'update' command, warning the user if it's not one of the supported
values 'checkout', 'merge', 'rebase' and 'none'. Please note that 'checkout'
update command was documented in upstream Documentation/gitmodules.txt [2] as
a valid 'submodule.name.update' value and code in upstream git-submodule.sh
implicitly assumes it as recognized value. Documentation/git-submodule.txt
doesn't mention it properly and I guess this is the reason why it wasn't
considered for the validation in [1] patch.

Using this patch, the previous workflow becomes:

# Maintainer
$ git