Re: [PATCH v2 04/23] contrib/subtree: Teach push and pull to use .gittrees for defaults

2013-03-24 Thread Paul Campbell
On Mon, Mar 11, 2013 at 3:35 AM, Junio C Hamano  wrote:
>> From: bibendi 
>>
>> Look in the config file .gittrees for a default repository and
>> refspec or commit when they are not provided on the command line.
>>
>> Uses the .gittrees config file in a similar way to how git-submodule
>> uses the .gitmodules file.
>
> What the patch does can be read from the code, but what benefit
> would users get by the extra file?

How about:

"Usually push and pull are to the same repository/branch that they were
originally added from. Add stores the repository/branch in .gittrees
which push and pull can now default to if not provided on the command
line."

>>  cmd_pull()
>>  {
>> - ensure_clean
>> - git fetch "$@" || exit $?
>> - revs=FETCH_HEAD
>> - set -- $revs
>> - cmd_merge "$@"
>> +if [ $# -ne 1 ]; then
>
> Broken indentation?
>
>> + die "You must provide "
>> + fi
>
> It used to allow "git fetch $there" and let the configured
> remote.$there.fetch refspec to decide what gets fetched, and also it
> used to allow "git fetch $there $that_branch" to explicitly fetch
> the named branch.  But this change insists that the user has to give
> what gets fetched from the command line and forbids the user from
> giving where to fetch from, it seems.  Isn't it a regression?  Why
> is it a good idea to forbid such uses that the script used to
> accept?
>
> The proposed log message does not explain why it is not a
> regression, or why accepting some use patterns that the script used
> to allow was a bug that needs to be diagnosed with this new
> conditional.

I think the patch was based on an older version of git-subtree before
"git fetch $there" support was added. Will need to update it.

>> + if [ -e "$dir" ]; then
>> + ensure_clean
>> + repository=$(git config -f .gittrees subtree.$prefix.url)
>> + refspec=$1
>> + git fetch $repository $refspec || exit $?
>> + echo "git fetch using: " $repository $refspec
>
> Why are these variable references outside the dq pair?
>

They're inside now.

Rerolling once I figure out the update for "git fetch $there" support.

-- 
Paul [W] Campbell
--
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 04/23] contrib/subtree: Teach push and pull to use .gittrees for defaults

2013-03-10 Thread Junio C Hamano
> From: bibendi 
>
> Look in the config file .gittrees for a default repository and
> refspec or commit when they are not provided on the command line.
>
> Uses the .gittrees config file in a similar way to how git-submodule
> uses the .gitmodules file.

What the patch does can be read from the code, but what benefit
would users get by the extra file?

>  cmd_pull()
>  {
> - ensure_clean
> - git fetch "$@" || exit $?
> - revs=FETCH_HEAD
> - set -- $revs
> - cmd_merge "$@"
> +if [ $# -ne 1 ]; then

Broken indentation?

> + die "You must provide "
> + fi

It used to allow "git fetch $there" and let the configured
remote.$there.fetch refspec to decide what gets fetched, and also it
used to allow "git fetch $there $that_branch" to explicitly fetch
the named branch.  But this change insists that the user has to give
what gets fetched from the command line and forbids the user from
giving where to fetch from, it seems.  Isn't it a regression?  Why
is it a good idea to forbid such uses that the script used to
accept?

The proposed log message does not explain why it is not a
regression, or why accepting some use patterns that the script used
to allow was a bug that needs to be diagnosed with this new
conditional.

> + if [ -e "$dir" ]; then
> + ensure_clean
> + repository=$(git config -f .gittrees subtree.$prefix.url)
> + refspec=$1
> + git fetch $repository $refspec || exit $?
> + echo "git fetch using: " $repository $refspec

Why are these variable references outside the dq pair?

--
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 v2 04/23] contrib/subtree: Teach push and pull to use .gittrees for defaults

2013-03-10 Thread Paul Campbell
From: bibendi 

Look in the config file .gittrees for a default repository and
refspec or commit when they are not provided on the command line.

Uses the .gittrees config file in a similar way to how git-submodule
uses the .gitmodules file.

Signed-off-by: Paul Campbell 
---
 contrib/subtree/git-subtree.sh | 28 +++-
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7b70251..1aff956 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -708,21 +708,31 @@ cmd_merge()
 
 cmd_pull()
 {
-   ensure_clean
-   git fetch "$@" || exit $?
-   revs=FETCH_HEAD
-   set -- $revs
-   cmd_merge "$@"
+if [ $# -ne 1 ]; then
+   die "You must provide "
+   fi
+   if [ -e "$dir" ]; then
+   ensure_clean
+   repository=$(git config -f .gittrees subtree.$prefix.url)
+   refspec=$1
+   git fetch $repository $refspec || exit $?
+   echo "git fetch using: " $repository $refspec
+   revs=FETCH_HEAD
+   set -- $revs
+   cmd_merge "$@"
+   else
+   die "'$dir' must already exist. Try 'git subtree add'."
+   fi
 }
 
 cmd_push()
 {
-   if [ $# -ne 2 ]; then
-   die "You must provide  "
+   if [ $# -ne 1 ]; then
+   die "You must provide "
fi
if [ -e "$dir" ]; then
-   repository=$1
-   refspec=$2
+   repository=$(git config -f .gittrees subtree.$prefix.url)
+   refspec=$1
echo "git push using: " $repository $refspec
git push $repository $(git subtree split 
--prefix=$prefix):refs/heads/$refspec
else
-- 
1.8.2.rc1

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