git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: greened@obbligato.org (David A. Greene)
To: Mathias Nyman <mathias.nyman@iki.fi>
Cc: git@vger.kernel.org, gitster@pobox.com, npaolucci@atlassian.com
Subject: Re: [PATCH] contrib/subtree: add repo url to commit messages
Date: Sat, 21 May 2016 17:52:11 -0500	[thread overview]
Message-ID: <87twhrowh0.fsf@waller.obbligato.org> (raw)
In-Reply-To: <20160223102559.GA18668@iki.fi> (Mathias Nyman's message of "Tue, 23 Feb 2016 12:25:59 +0200")

Mathias Nyman <mathias.nyman@iki.fi> writes:

> For recalling where a subtree came from; git-subtree operations 'add'
> and 'pull', when called with the <repository> parameter add this to the
> commit message:
>     git-subtree-repo: <repo_url>

I am sorry it tooks a couple of months to respond.  I am finally coming
up for air at work.

What is the future intent of this?  I've toyed with the idea of adding
something like this either as commit message metadata or in .gitconfig
but every time I get ready to pull the trigger, I question what it will
be used for.

Having been using git-subtree in anger for a couple of years now, I
frequently pull subtree updates from multiple sources, so noting a
particular repository is not only mostly meaningless, it may actually be
misleading in that a quick perusal of the logs may lead one to think
commits were draw from fewer places than they actually were.

I don't think it would be a good idea, for example, to have git-subtree
use this information to auto-guess from where to pull future commits.
Again, I think that would be misleading behavior.

                         -David

> Other operations that don't have the <repository> information, like
> 'merge' and 'add' without <repository>, are unchanged. Users with such a
> workflow will continue to be on their own with the --message parameter,
> if they'd like to record where the subtree came from.
>
> Signed-off-by: Mathias Nyman <mathias.nyman@iki.fi>
> Based-on-patch-by: Nicola Paolucci <npaolucci@atlassian.com>
> ---
>  contrib/subtree/git-subtree.sh | 73 ++++++++++++++++++++++++++++--------------
>  1 file changed, 49 insertions(+), 24 deletions(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 7a39b30..7cf73c0 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -335,18 +335,21 @@ add_msg()
>  	dir="$1"
>  	latest_old="$2"
>  	latest_new="$3"
> +	repo="$4" # optional
>  	if [ -n "$message" ]; then
>  		commit_message="$message"
>  	else
>  		commit_message="Add '$dir/' from commit '$latest_new'"
>  	fi
> -	cat <<-EOF
> -		$commit_message
> -		
> -		git-subtree-dir: $dir
> -		git-subtree-mainline: $latest_old
> -		git-subtree-split: $latest_new
> -	EOF
> +	echo $commit_message
> +	echo
> +	echo git-subtree-dir: $dir
> +	echo git-subtree-mainline: $latest_old
> +	echo git-subtree-split: $latest_new
> +	if [ -n "$repo" ]; then
> +		repo_url=$(get_repository_url "$repo")
> +		echo "git-subtree-repo: $repo_url"
> +	fi
>  }
>  
>  add_squashed_msg()
> @@ -382,8 +385,9 @@ squash_msg()
>  	dir="$1"
>  	oldsub="$2"
>  	newsub="$3"
> +	repo="$4" # optional
>  	newsub_short=$(git rev-parse --short "$newsub")
> -	
> +
>  	if [ -n "$oldsub" ]; then
>  		oldsub_short=$(git rev-parse --short "$oldsub")
>  		echo "Squashed '$dir/' changes from $oldsub_short..$newsub_short"
> @@ -397,6 +401,10 @@ squash_msg()
>  	echo
>  	echo "git-subtree-dir: $dir"
>  	echo "git-subtree-split: $newsub"
> +	if [ -n "$repo" ]; then
> +		repo_url=$(get_repository_url "$repo")
> +		echo "git-subtree-repo: $repo_url"
> +	fi
>  }
>  
>  toptree_for_commit()
> @@ -440,12 +448,13 @@ new_squash_commit()
>  	old="$1"
>  	oldsub="$2"
>  	newsub="$3"
> +	repo="$4" # optional
>  	tree=$(toptree_for_commit $newsub) || exit $?
>  	if [ -n "$old" ]; then
> -		squash_msg "$dir" "$oldsub" "$newsub" | 
> +		squash_msg "$dir" "$oldsub" "$newsub" "$repo" |
>  			git commit-tree "$tree" -p "$old" || exit $?
>  	else
> -		squash_msg "$dir" "" "$newsub" |
> +		squash_msg "$dir" "" "$newsub" "$repo" |
>  			git commit-tree "$tree" || exit $?
>  	fi
>  }
> @@ -517,6 +526,16 @@ ensure_valid_ref_format()
>  	    die "'$1' does not look like a ref"
>  }
>  
> +get_repository_url()
> +{
> +	repo=$1
> +	repo_url=$(git config --get remote.$repo.url)
> +	if [ -z "$repo_url" ]; then
> +		repo_url=$repo
> +	fi
> +	echo $repo_url
> +}
> +
>  cmd_add()
>  {
>  	if [ -e "$dir" ]; then
> @@ -548,19 +567,18 @@ cmd_add()
>  cmd_add_repository()
>  {
>  	echo "git fetch" "$@"
> -	repository=$1
> +	repo=$1
>  	refspec=$2
>  	git fetch "$@" || exit $?
>  	revs=FETCH_HEAD
> -	set -- $revs
> +	set -- $revs $repo
>  	cmd_add_commit "$@"
>  }
>  
>  cmd_add_commit()
>  {
> -	revs=$(git rev-parse $default --revs-only "$@") || exit $?
> -	set -- $revs
> -	rev="$1"
> +	rev=$(git rev-parse $default --revs-only "$1") || exit $?
> +	repo="$2" # optional
>  	
>  	debug "Adding $dir as '$rev'..."
>  	git read-tree --prefix="$dir" $rev || exit $?
> @@ -575,12 +593,12 @@ cmd_add_commit()
>  	fi
>  	
>  	if [ -n "$squash" ]; then
> -		rev=$(new_squash_commit "" "" "$rev") || exit $?
> +		rev=$(new_squash_commit "" "" "$rev" "$repo") || exit $?
>  		commit=$(add_squashed_msg "$rev" "$dir" |
>  			 git commit-tree $tree $headp -p "$rev") || exit $?
>  	else
>  		revp=$(peel_committish "$rev") &&
> -		commit=$(add_msg "$dir" "$headrev" "$rev" |
> +		commit=$(add_msg "$dir" "$headrev" "$rev" "$repo" |
>  			 git commit-tree $tree $headp -p "$revp") || exit $?
>  	fi
>  	git reset "$commit" || exit $?
> @@ -609,7 +627,8 @@ cmd_split()
>  	else
>  		unrevs="$(find_existing_splits "$dir" "$revs")"
>  	fi
> -	
> +e
> +	rev="$1"
>  	# We can't restrict rev-list to only $dir here, because some of our
>  	# parents have the $dir contents the root, and those won't match.
>  	# (and rev-list --follow doesn't seem to solve this)
> @@ -683,15 +702,20 @@ cmd_split()
>  
>  cmd_merge()
>  {
> -	revs=$(git rev-parse $default --revs-only "$@") || exit $?
> +	revs=$(git rev-parse $default --revs-only "$1") || exit $?
>  	ensure_clean
> -	
>  	set -- $revs
>  	if [ $# -ne 1 ]; then
>  		die "You must provide exactly one revision.  Got: '$revs'"
>  	fi
> +	do_merge "$@"
> +}
> +
> +do_merge()
> +{
>  	rev="$1"
> -	
> +	repo="$2" # optional
> +
>  	if [ -n "$squash" ]; then
>  		first_split="$(find_latest_squash "$dir")"
>  		if [ -z "$first_split" ]; then
> @@ -704,7 +728,7 @@ cmd_merge()
>  			say "Subtree is already at commit $rev."
>  			exit 0
>  		fi
> -		new=$(new_squash_commit "$old" "$sub" "$rev") || exit $?
> +		new=$(new_squash_commit "$old" "$sub" "$rev" "$repo") || exit $?
>  		debug "New squash commit: $new"
>  		rev="$new"
>  	fi
> @@ -730,12 +754,13 @@ cmd_pull()
>  	if [ $# -ne 2 ]; then
>  	    die "You must provide <repository> <ref>"
>  	fi
> +	repo=$1
>  	ensure_clean
>  	ensure_valid_ref_format "$2"
>  	git fetch "$@" || exit $?
>  	revs=FETCH_HEAD
> -	set -- $revs
> -	cmd_merge "$@"
> +	set -- $revs $repo
> +	do_merge "$@"
>  }
>  
>  cmd_push()

  parent reply	other threads:[~2016-05-21 23:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-23 10:25 [PATCH] contrib/subtree: add repo url to commit messages Mathias Nyman
2016-02-25 22:23 ` Eric Sunshine
2016-02-26  8:28   ` Mathias Nyman
2016-02-26 19:49     ` Eric Sunshine
2016-03-03 11:42       ` Mathias Nyman
2016-05-21 22:52 ` David A. Greene [this message]
2016-05-31 12:19   ` Mathias Nyman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87twhrowh0.fsf@waller.obbligato.org \
    --to=greened@obbligato.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mathias.nyman@iki.fi \
    --cc=npaolucci@atlassian.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).