* [PATCH/RFC 0/3] Split fetch and merge logic
@ 2007-02-16 8:06 Santi Béjar
2007-02-16 8:09 ` [PATCH/RFC 2/3] git-fetch: " Santi Béjar
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Santi Béjar @ 2007-02-16 8:06 UTC (permalink / raw)
To: Git Mailing List
This series implements the split of the fetch and merge logic.
The first patch adds tests for the merge logic. It is very extensive and
expensive (in time), so I just post this to show that (almost) nothing
changes (although some part of if could be included). And the third
patch shows what changes.
Santi Béjar (3):
t/t5515-fetch-merge-logic.sh: Added tests for the merge login in git-fetch
git-fetch: Split fetch and merge logic
t/t5515: fixes for the separate fetch and merge logic
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH/RFC 2/3] git-fetch: Split fetch and merge logic 2007-02-16 8:06 [PATCH/RFC 0/3] Split fetch and merge logic Santi Béjar @ 2007-02-16 8:09 ` Santi Béjar 2007-02-19 20:44 ` Junio C Hamano 2007-02-16 8:10 ` [PATCH/RFC 3/3] t/t5515: fixes for the separate " Santi Béjar ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: Santi Béjar @ 2007-02-16 8:09 UTC (permalink / raw) To: Git Mailing List git-fetch fetches the branches from the remote and saves this information in .git/FETCH_FETCHED, and at the end it generates the file .git/FETCH_HEAD. There are two cases where the behaviour is changed: 1) branch.*.merge no longer must exactly match the remote part of the branch fetched. Both are expanded in full (as refs/heads/...) and matched afterwards. 2) When the remote is specified with $GIT_DIR/branches/... and there is a branch.*.merge, the remote branch name must match to get them merged. Before the branch in $GIT_DIR/branches/... was always merged. In the documentation the $GIT_DIR/branches/ is documented as a short-hand for a corresponding file in $GIT_DIR/remotes/, so I think this makes the new behaviour consistent. Signed-off-by: Santi Béjar <sbejar@gmail.com> --- Documentation/config.txt | 2 +- git-fetch.sh | 74 ++++++++++++++++++++++++++------------------- git-parse-remote.sh | 60 +++++++++---------------------------- t/t5510-fetch.sh | 16 ++++++++++ 4 files changed, 75 insertions(+), 77 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 9620126..e695de5 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -238,7 +238,7 @@ branch.<name>.remote:: branch.<name>.merge:: When in branch <name>, it tells `git fetch` the default refspec to - be marked for merging in FETCH_HEAD. The value has exactly to match + be marked for merging in FETCH_HEAD. The value has to match a remote part of one of the refspecs which are fetched from the remote given by "branch.<name>.remote". The merge information is used by `git pull` (which at first calls diff --git a/git-fetch.sh b/git-fetch.sh index ca984e7..727538d 100755 --- a/git-fetch.sh +++ b/git-fetch.sh @@ -77,9 +77,9 @@ do shift done +origin=$(get_default_remote) case "$#" in 0) - origin=$(get_default_remote) test -n "$(get_remote_url ${origin})" || die "Where do you want to fetch from today?" set x $origin ; shift ;; @@ -101,6 +101,7 @@ if test "" = "$append" then : >"$GIT_DIR/FETCH_HEAD" fi +: >"$GIT_DIR/FETCH_FETCHED" # Global that is reused later ls_remote_result=$(git ls-remote $exec "$remote") || @@ -112,10 +113,6 @@ append_fetch_head () { remote_name_="$3" remote_nick_="$4" local_name_="$5" - case "$6" in - t) not_for_merge_='not-for-merge' ;; - '') not_for_merge_= ;; - esac # remote-nick is the URL given on the command line (or a shorthand) # remote-name is the $GIT_DIR relative refs/ path we computed @@ -146,9 +143,7 @@ append_fetch_head () { if git-cat-file commit "$head_" >/dev/null 2>&1 then headc_=$(git-rev-parse --verify "$head_^0") || exit - echo "$headc_ $not_for_merge_ $note_" >>"$GIT_DIR/FETCH_HEAD" - else - echo "$head_ not-for-merge $note_" >>"$GIT_DIR/FETCH_HEAD" + echo "$headc_ $remote_name_:$local_name_ $note_" >>"$GIT_DIR/FETCH_FETCHED" fi update_local_ref "$local_name_" "$head_" "$note_" @@ -256,7 +251,7 @@ then git-show-ref --exclude-existing=refs/tags/ | while read sha1 name do - echo ".${name}:${name}" + echo "${name}:${name}" done` || exit if test "$#" -gt 1 then @@ -279,13 +274,6 @@ fetch_main () { # These are relative path from $GIT_DIR, typically starting at refs/ # but may be HEAD - if expr "z$ref" : 'z\.' >/dev/null - then - not_for_merge=t - ref=$(expr "z$ref" : 'z\.\(.*\)') - else - not_for_merge= - fi if expr "z$ref" : 'z+' >/dev/null then single_force=t @@ -366,7 +354,7 @@ fetch_main () { esac append_fetch_head "$head" "$remote" \ - "$remote_name" "$remote_nick" "$local_name" "$not_for_merge" || exit + "$remote_name" "$remote_nick" "$local_name" || exit done @@ -409,28 +397,16 @@ fetch_main () { case "$ref" in +$remote_name:*) single_force=t - not_for_merge= - found="$ref" - break ;; - .+$remote_name:*) - single_force=t - not_for_merge=t - found="$ref" - break ;; - .$remote_name:*) - not_for_merge=t found="$ref" break ;; $remote_name:*) - not_for_merge= found="$ref" break ;; esac done local_name=$(expr "z$found" : 'z[^:]*:\(.*\)') append_fetch_head "$sha1" "$remote" \ - "$remote_name" "$remote_nick" "$local_name" \ - "$not_for_merge" || exit + "$remote_name" "$remote_nick" "$local_name" || exit done ) ) || exit ;; @@ -454,7 +430,7 @@ case "$no_tags$tags" in do git-cat-file -t "$sha1" >/dev/null 2>&1 || continue echo >&2 "Auto-following $name" - echo ".${name}:${name}" + echo "${name}:${name}" done) esac case "$taglist" in @@ -482,3 +458,39 @@ case "$orig_head" in fi ;; esac + +# Generate $GIT_DIR/FETCH_HEAD +case ",$#,$remote_nick," in +,1,$origin,) + curr_branch=$(git-symbolic-ref -q HEAD | sed -e 's|^refs/heads/||') + merge_branches=$(git-repo-config \ + --get-all "branch.${curr_branch}.merge") + [ -z "$merge_branches" ] && merge_first=yes;; +,1,$remote,) merge_branches=HEAD;; +,1,*)merge_first=yes ;; +*) + shift + merge_branches=$(for ref; do + expr "z$ref" : 'z.*:' >/dev/null || ref="${ref}:" + echo "$(expr "z$ref" : 'z\([^:]*\):')"; done);; +esac + +test "$merge_first" == "yes" && +test "$(get_remote_default_refs_for_fetch -t $remote_nick)" != "explicit" && +merge_branches= && merge_first= + +merge_branches=$(canon_refs_list_for_fetch $merge_branches | sed 's/:.*$//g') + +cat "$GIT_DIR"/FETCH_FETCHED | while IFS=' ' read sha1 ref note ; do + remote_branch=$(expr "z$ref" : 'z\([^:]*\):') + for merge_branch in $merge_branches ; do + [ "$merge_branch" == "$remote_branch" ] && + echo "$sha1 $note" && continue 2 + done + if ! test "$merge_first" || test "$merge_first" == "done" ; then + echo "$sha1 not-for-merge $note" + else + echo "$sha1 $note" + merge_first=done + fi +done >> "$GIT_DIR/FETCH_HEAD" diff --git a/git-parse-remote.sh b/git-parse-remote.sh index 5208ee6..212b3bc 100755 --- a/git-parse-remote.sh +++ b/git-parse-remote.sh @@ -70,8 +70,7 @@ get_remote_default_refs_for_push () { esac } -# Called from canon_refs_list_for_fetch -d "$remote", which -# is called from get_remote_default_refs_for_fetch to grok +# Called from get_remote_default_refs_for_fetch to grok # refspecs that are retrieved from the configuration, but not # from get_remote_refs_for_fetch when it deals with refspecs # supplied on the command line. $ls_remote_result has the list @@ -130,30 +129,6 @@ expand_refs_wildcard () { # Subroutine to canonicalize remote:local notation. canon_refs_list_for_fetch () { - # If called from get_remote_default_refs_for_fetch - # leave the branches in branch.${curr_branch}.merge alone, - # or the first one otherwise; add prefix . to the rest - # to prevent the secondary branches to be merged by default. - merge_branches= - curr_branch= - if test "$1" = "-d" - then - shift ; remote="$1" ; shift - set $(expand_refs_wildcard "$remote" "$@") - is_explicit="$1" - shift - if test "$remote" = "$(get_default_remote)" - then - curr_branch=$(git-symbolic-ref -q HEAD | \ - sed -e 's|^refs/heads/||') - merge_branches=$(git-config \ - --get-all "branch.${curr_branch}.merge") - fi - if test -z "$merge_branches" && test $is_explicit != explicit - then - merge_branches=..this.will.never.match.any.ref.. - fi - fi for ref do force= @@ -166,18 +141,6 @@ canon_refs_list_for_fetch () { expr "z$ref" : 'z.*:' >/dev/null || ref="${ref}:" remote=$(expr "z$ref" : 'z\([^:]*\):') local=$(expr "z$ref" : 'z[^:]*:\(.*\)') - dot_prefix=. - if test -z "$merge_branches" - then - merge_branches=$remote - dot_prefix= - else - for merge_branch in $merge_branches - do - [ "$remote" = "$merge_branch" ] && - dot_prefix= && break - done - fi case "$remote" in '' | HEAD ) remote=HEAD ;; refs/heads/* | refs/tags/* | refs/remotes/*) ;; @@ -196,32 +159,39 @@ canon_refs_list_for_fetch () { git-check-ref-format "$local_ref_name" || die "* refusing to create funny ref '$local_ref_name' locally" fi - echo "${dot_prefix}${force}${remote}:${local}" + echo "${force}${remote}:${local}" done } # Returns list of src: (no store), or src:dst (store) get_remote_default_refs_for_fetch () { + test "$1" == -t && type=yes && shift data_source=$(get_data_source "$1") case "$data_source" in '') - echo "HEAD:" ;; + set explicit "HEAD:" ;; config) - canon_refs_list_for_fetch -d "$1" \ - $(git-config --get-all "remote.$1.fetch") ;; + set $(expand_refs_wildcard "$1" \ + $(git-repo-config --get-all "remote.$1.fetch")) ;; branches) remote_branch=$(sed -ne '/#/s/.*#//p' "$GIT_DIR/branches/$1") case "$remote_branch" in '') remote_branch=master ;; esac - echo "refs/heads/${remote_branch}:refs/heads/$1" + set explicit "refs/heads/${remote_branch}:refs/heads/$1" ;; remotes) - canon_refs_list_for_fetch -d "$1" $(sed -ne '/^Pull: */{ + set $(expand_refs_wildcard "$1" $(sed -ne '/^Pull: */{ s///p - }' "$GIT_DIR/remotes/$1") + }' "$GIT_DIR/remotes/$1")) ;; *) die "internal error: get-remote-default-ref-for-push $1" ;; esac + if [ "$type" ] ; then + echo $1 + else + shift + canon_refs_list_for_fetch "$@" + fi } get_remote_refs_for_push () { diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 50c6485..0a19a7d 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -50,6 +50,22 @@ test_expect_success "fetch test" ' test "z$mine" = "z$his" ' +test_expect_success "fetch test fetched" ' + cd "$D" && + cd three && + git fetch && + test -f .git/refs/heads/two && + test -f .git/refs/heads/one && + master_in_two=`cd ../two && git rev-parse master` && + one_in_two=`cd ../two && git rev-parse one` && + { + echo "$master_in_two refs/heads/master:refs/heads/two" + echo "$one_in_two refs/heads/one:refs/heads/one" + } >expected && + cut -f -2 .git/FETCH_FETCHED >actual && + diff expected actual' + + test_expect_success "fetch test for-merge" ' cd "$D" && cd three && -- 1.5.0.35.gaaba ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH/RFC 2/3] git-fetch: Split fetch and merge logic 2007-02-16 8:09 ` [PATCH/RFC 2/3] git-fetch: " Santi Béjar @ 2007-02-19 20:44 ` Junio C Hamano 2007-02-19 22:13 ` Santi Béjar 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2007-02-19 20:44 UTC (permalink / raw) To: Santi Béjar; +Cc: Git Mailing List Santi Béjar <sbejar@gmail.com> writes: > git-fetch fetches the branches from the remote and saves this > information in .git/FETCH_FETCHED, and at the end it generates > the file .git/FETCH_HEAD. > > There are two cases where the behaviour is changed: > > 1) branch.*.merge no longer must exactly match the remote part > of the branch fetched. Both are expanded in full (as refs/heads/...) > and matched afterwards. How hard would it be to fix this? I see this as a regression. If you are setting configuration, wouldn't you rather see the behaviour consistent even when remote adds new refs? > 2) When the remote is specified with $GIT_DIR/branches/... and there is > a branch.*.merge, the remote branch name must match to get them merged. > Before the branch in $GIT_DIR/branches/... was always merged. I do not think the current $GIT_DIR/branches/ support with respect to choosing which remote branch to choose was done with any deep thinking, other than to stay backward compatible, so I would not put too much trust in what is in the documentation. At the same time, I personally can be pursuaded to go either way, exactly because I do not think the current behaviour has strict reasoning behind it. However, I wonder how this change would affect existing setups people may have. Merging this at this moment would be a pain even if there were no downsides, as there are a few topics that want to touch parse-remote and fetch (two already in 'pu', and git-bundle series also wants to hook into git-fetch); these topics would need to get adjusted if this clean-up goes in first. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH/RFC 2/3] git-fetch: Split fetch and merge logic 2007-02-19 20:44 ` Junio C Hamano @ 2007-02-19 22:13 ` Santi Béjar 2007-02-19 23:27 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Santi Béjar @ 2007-02-19 22:13 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List On 2/19/07, Junio C Hamano <junkio@cox.net> wrote: > Santi Béjar <sbejar@gmail.com> writes: > > > git-fetch fetches the branches from the remote and saves this > > information in .git/FETCH_FETCHED, and at the end it generates > > the file .git/FETCH_HEAD. > > > > There are two cases where the behaviour is changed: > > > > 1) branch.*.merge no longer must exactly match the remote part > > of the branch fetched. Both are expanded in full (as refs/heads/...) > > and matched afterwards. > > How hard would it be to fix this? At least it is not a 2 lines "fix". > I see this as a regression. > If you are setting configuration, wouldn't you rather see the > behaviour consistent even when remote adds new refs? I don't see it a regression. All current setups continue to work properly and, sorry but I don't see how adding a new ref changes this. > > > 2) When the remote is specified with $GIT_DIR/branches/... and there is > > a branch.*.merge, the remote branch name must match to get them merged. > > Before the branch in $GIT_DIR/branches/... was always merged. > > I do not think the current $GIT_DIR/branches/ support with > respect to choosing which remote branch to choose was done with > any deep thinking, other than to stay backward compatible, so I > would not put too much trust in what is in the documentation. > At the same time, I personally can be pursuaded to go either > way, exactly because I do not think the current behaviour has > strict reasoning behind it. I prefer my way, but I don't mind much either and it can be "fixed". > > However, I wonder how this change would affect existing setups > people may have. > > Merging this at this moment would be a pain even if there were > no downsides, as there are a few topics that want to touch > parse-remote and fetch (two already in 'pu', and git-bundle > series also wants to hook into git-fetch); these topics would > need to get adjusted if this clean-up goes in first. A problematic decision :) Santi ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH/RFC 2/3] git-fetch: Split fetch and merge logic 2007-02-19 22:13 ` Santi Béjar @ 2007-02-19 23:27 ` Junio C Hamano 2007-02-20 11:21 ` Santi Béjar 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2007-02-19 23:27 UTC (permalink / raw) To: Santi Béjar; +Cc: Junio C Hamano, Git Mailing List "Santi Béjar" <sbejar@gmail.com> writes: >> > There are two cases where the behaviour is changed: >> > >> > 1) branch.*.merge no longer must exactly match the remote part >> > of the branch fetched. Both are expanded in full (as refs/heads/...) >> > and matched afterwards. > ... >> I see this as a regression. >> If you are setting configuration, wouldn't you rather see the >> behaviour consistent even when remote adds new refs? Maybe I misread your description, but I took it to mean that you are allowing: branch.master.merge = a to mean what we traditionally spelled branch.master.merge = refs/heads/a and guessed (I haven't looked for where it happens in the code) the way you do that conversion is by tail-matching the ref; if the other end creates "refs/heads/b/a", suddenly remote branch b/a starts matching that pattern wouldn't it? Earlier we fixed the ambiguous use of branch.*.merge in 756373da; I think the same reasoning should apply here. Configuration is something you set once because you want to forget about it afterwards (iow, not having to type every time), and I think making sure it names things unambiguously outweighs one-time convenience of being able to write the configuration in a looser fashion. If somebody does "git checkout -B origin/next" which does: git checkout -b next origin/next && git repo-config branch.next.merge $merge I would expect that the enhanced "checkout" script would not have any trouble arranging $merge to fully spell out refs/heads/next. >> Merging this at this moment would be a pain even if there were >> no downsides, as there are a few topics that want to touch >> parse-remote and fetch (two already in 'pu', and git-bundle >> series also wants to hook into git-fetch); these topics would >> need to get adjusted if this clean-up goes in first. > > A problematic decision :) Not at all. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH/RFC 2/3] git-fetch: Split fetch and merge logic 2007-02-19 23:27 ` Junio C Hamano @ 2007-02-20 11:21 ` Santi Béjar 0 siblings, 0 replies; 14+ messages in thread From: Santi Béjar @ 2007-02-20 11:21 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List On 2/20/07, Junio C Hamano <junkio@cox.net> wrote: > "Santi Béjar" <sbejar@gmail.com> writes: > > >> > There are two cases where the behaviour is changed: > >> > > >> > 1) branch.*.merge no longer must exactly match the remote part > >> > of the branch fetched. Both are expanded in full (as refs/heads/...) > >> > and matched afterwards. > > ... > >> I see this as a regression. > >> If you are setting configuration, wouldn't you rather see the > >> behaviour consistent even when remote adds new refs? > > Maybe I misread your description, but I took it to mean that you > are allowing: > > branch.master.merge = a > > to mean what we traditionally spelled > > branch.master.merge = refs/heads/a > > and guessed (I haven't looked for where it happens in the code) > the way you do that conversion is by tail-matching the ref; if > the other end creates "refs/heads/b/a", suddenly remote branch > b/a starts matching that pattern wouldn't it? No. branch.master.merge = a is equivalent to refs/heads/a and only matches with the remote branch refs/heads/a. It continues to exactly match the two branches, but with the full patch (refs/...). So now it is possible to have: [remote "origin"] url = ... fetch = refs/heads/*:refs/heads/origin/* [branch "master"] remote = origin merge = master or the other way: [remote "origin"] url = ... fetch = master:refs/heads/origin [branch "master"] remote = origin merge = refs/heads/master > > Earlier we fixed the ambiguous use of branch.*.merge in > 756373da; I think the same reasoning should apply here. > > Configuration is something you set once because you want to > forget about it afterwards (iow, not having to type every time), > and I think making sure it names things unambiguously outweighs > one-time convenience of being able to write the configuration in > a looser fashion. It is unambiguous. But if it is problematic I'll try to keep the current behaviour. Santi ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH/RFC 3/3] t/t5515: fixes for the separate fetch and merge logic 2007-02-16 8:06 [PATCH/RFC 0/3] Split fetch and merge logic Santi Béjar 2007-02-16 8:09 ` [PATCH/RFC 2/3] git-fetch: " Santi Béjar @ 2007-02-16 8:10 ` Santi Béjar 2007-02-16 8:22 ` [PATCH/RFC 0/3] Split " Junio C Hamano [not found] ` <87zm7eo78x.fsf@gmail.com> 3 siblings, 0 replies; 14+ messages in thread From: Santi Béjar @ 2007-02-16 8:10 UTC (permalink / raw) To: Git Mailing List 1) branch.*.merge no longer must exactly match the remote part of the branch fetched. Both are expanded in full (as refs/heads/...) and matched afterwards. 2) When the remote is specified with $GIT_DIR/branches/... and there is a branch.*.merge, the remote branch name must match to get them merged. Before the branch in $GIT_DIR/branches/... was always merged. In the documentation the $GIT_DIR/branches/ is documented as a short-hand for a corresponding file in $GIT_DIR/remotes/, so I think this makes the new behaviour consistent. Signed-off-by: Santi Béjar <sbejar@gmail.com> --- t/t5515/fetch.br-branches-default-merge | 2 +- ...etch.br-branches-default-merge_branches-default | 2 +- t/t5515/fetch.br-branches-default-octopus | 2 +- ...ch.br-branches-default-octopus_branches-default | 2 +- t/t5515/fetch.br-branches-one-merge | 2 +- t/t5515/fetch.br-branches-one-merge_branches-one | 2 +- t/t5515/fetch.br-config-glob-octopus | 2 +- t/t5515/fetch.br-config-glob-octopus_config-glob | 2 +- t/t5515/fetch.br-remote-glob-octopus | 2 +- t/t5515/fetch.br-remote-glob-octopus_remote-glob | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/t/t5515/fetch.br-branches-default-merge b/t/t5515/fetch.br-branches-default-merge index ca85356..28b85e1 100644 --- a/t/t5515/fetch.br-branches-default-merge +++ b/t/t5515/fetch.br-branches-default-merge @@ -1,2 +1,2 @@ # br-branches-default-merge -754b754407bf032e9a2f9d5a9ad05ca79a6b228f +754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge diff --git a/t/t5515/fetch.br-branches-default-merge_branches-default b/t/t5515/fetch.br-branches-default-merge_branches-default index cb5acac..1d86ad1 100644 --- a/t/t5515/fetch.br-branches-default-merge_branches-default +++ b/t/t5515/fetch.br-branches-default-merge_branches-default @@ -1,2 +1,2 @@ # br-branches-default-merge branches-default -754b754407bf032e9a2f9d5a9ad05ca79a6b228f +754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge diff --git a/t/t5515/fetch.br-branches-default-octopus b/t/t5515/fetch.br-branches-default-octopus index 002b82e..22bed59 100644 --- a/t/t5515/fetch.br-branches-default-octopus +++ b/t/t5515/fetch.br-branches-default-octopus @@ -1,2 +1,2 @@ # br-branches-default-octopus -754b754407bf032e9a2f9d5a9ad05ca79a6b228f +754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge diff --git a/t/t5515/fetch.br-branches-default-octopus_branches-default b/t/t5515/fetch.br-branches-default-octopus_branches-default index 7443cf7..c584f16 100644 --- a/t/t5515/fetch.br-branches-default-octopus_branches-default +++ b/t/t5515/fetch.br-branches-default-octopus_branches-default @@ -1,2 +1,2 @@ # br-branches-default-octopus branches-default -754b754407bf032e9a2f9d5a9ad05ca79a6b228f +754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge diff --git a/t/t5515/fetch.br-branches-one-merge b/t/t5515/fetch.br-branches-one-merge index e594bec..c7f5013 100644 --- a/t/t5515/fetch.br-branches-one-merge +++ b/t/t5515/fetch.br-branches-one-merge @@ -1,2 +1,2 @@ # br-branches-one-merge -8e32a6d901327a23ef831511badce7bf3bf46689 +8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge diff --git a/t/t5515/fetch.br-branches-one-merge_branches-one b/t/t5515/fetch.br-branches-one-merge_branches-one index 168031c..1473f5c 100644 --- a/t/t5515/fetch.br-branches-one-merge_branches-one +++ b/t/t5515/fetch.br-branches-one-merge_branches-one @@ -1,2 +1,2 @@ # br-branches-one-merge branches-one -8e32a6d901327a23ef831511badce7bf3bf46689 +8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge diff --git a/t/t5515/fetch.br-config-glob-octopus b/t/t5515/fetch.br-config-glob-octopus index da95823..0f324e0 100644 --- a/t/t5515/fetch.br-config-glob-octopus +++ b/t/t5515/fetch.br-config-glob-octopus @@ -2,4 +2,4 @@ 754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge 8e32a6d901327a23ef831511badce7bf3bf46689 0567da4d5edd2ff4bb292a465ba9e64dcad9536b not-for-merge -6134ee8f857693b96ff1cc98d3e2fd62b199e5a8 not-for-merge +6134ee8f857693b96ff1cc98d3e2fd62b199e5a8 diff --git a/t/t5515/fetch.br-config-glob-octopus_config-glob b/t/t5515/fetch.br-config-glob-octopus_config-glob index 0e7c769..8151e4c 100644 --- a/t/t5515/fetch.br-config-glob-octopus_config-glob +++ b/t/t5515/fetch.br-config-glob-octopus_config-glob @@ -2,4 +2,4 @@ 754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge 8e32a6d901327a23ef831511badce7bf3bf46689 0567da4d5edd2ff4bb292a465ba9e64dcad9536b not-for-merge -6134ee8f857693b96ff1cc98d3e2fd62b199e5a8 not-for-merge +6134ee8f857693b96ff1cc98d3e2fd62b199e5a8 diff --git a/t/t5515/fetch.br-remote-glob-octopus b/t/t5515/fetch.br-remote-glob-octopus index 92ef25c..4a14384 100644 --- a/t/t5515/fetch.br-remote-glob-octopus +++ b/t/t5515/fetch.br-remote-glob-octopus @@ -2,4 +2,4 @@ 754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge 8e32a6d901327a23ef831511badce7bf3bf46689 0567da4d5edd2ff4bb292a465ba9e64dcad9536b not-for-merge -6134ee8f857693b96ff1cc98d3e2fd62b199e5a8 not-for-merge +6134ee8f857693b96ff1cc98d3e2fd62b199e5a8 diff --git a/t/t5515/fetch.br-remote-glob-octopus_remote-glob b/t/t5515/fetch.br-remote-glob-octopus_remote-glob index 6ad3726..7f6f928 100644 --- a/t/t5515/fetch.br-remote-glob-octopus_remote-glob +++ b/t/t5515/fetch.br-remote-glob-octopus_remote-glob @@ -2,4 +2,4 @@ 754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge 8e32a6d901327a23ef831511badce7bf3bf46689 0567da4d5edd2ff4bb292a465ba9e64dcad9536b not-for-merge -6134ee8f857693b96ff1cc98d3e2fd62b199e5a8 not-for-merge +6134ee8f857693b96ff1cc98d3e2fd62b199e5a8 -- 1.5.0.35.gaaba ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH/RFC 0/3] Split fetch and merge logic 2007-02-16 8:06 [PATCH/RFC 0/3] Split fetch and merge logic Santi Béjar 2007-02-16 8:09 ` [PATCH/RFC 2/3] git-fetch: " Santi Béjar 2007-02-16 8:10 ` [PATCH/RFC 3/3] t/t5515: fixes for the separate " Santi Béjar @ 2007-02-16 8:22 ` Junio C Hamano 2007-02-16 8:40 ` Santi Béjar [not found] ` <87zm7eo78x.fsf@gmail.com> 3 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2007-02-16 8:22 UTC (permalink / raw) To: Santi Béjar; +Cc: Git Mailing List Santi Béjar <sbejar@gmail.com> writes: > This series implements the split of the fetch and merge logic. That's a description of what it does, but could you describe what problem it solves, please? The approach of presentation to introduce the test and showing the behavior change in a later patch is very good, though. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH/RFC 0/3] Split fetch and merge logic 2007-02-16 8:22 ` [PATCH/RFC 0/3] Split " Junio C Hamano @ 2007-02-16 8:40 ` Santi Béjar 2007-02-16 20:10 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Santi Béjar @ 2007-02-16 8:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List On 2/16/07, Junio C Hamano <junkio@cox.net> wrote: > Santi Béjar <sbejar@gmail.com> writes: > > > This series implements the split of the fetch and merge logic. > > That's a description of what it does, but could you describe > what problem it solves, please? Sure. In the past we've had problems when we have changed the merge logic (as shows 4363dfbe3). This patch makes the two process completely independent, and concentrate the merge logic in one place (leaving git-parse-remote.sh independent of the merge logic). > > The approach of presentation to introduce the test and showing > the behavior change in a later patch is very good, though. Thanks. Santi ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH/RFC 0/3] Split fetch and merge logic 2007-02-16 8:40 ` Santi Béjar @ 2007-02-16 20:10 ` Junio C Hamano 2007-02-16 20:30 ` Santi Béjar 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2007-02-16 20:10 UTC (permalink / raw) To: Santi Béjar; +Cc: Junio C Hamano, Git Mailing List "Santi Béjar" <sbejar@gmail.com> writes: > In the past we've had problems when we have changed the merge logic > (as shows 4363dfbe3). This patch makes the two process completely > independent, and concentrate the merge logic in one place (leaving > git-parse-remote.sh independent of the merge logic). But that is a solved problem, isn't it? What else does it solve? The justification for moving around the logic could be something like "these three patches do not do that themselves, but it opens a door for further work such as ...", but without something concrete in "..." part, your response makes the patch look mostly needless code churn. I was hoping to hear something like "now git-fetch has to do much less than before, eventual C-rewrite of the command, which can borrow some code already written for 'pu' branch, will become much easier" ;-). ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH/RFC 0/3] Split fetch and merge logic 2007-02-16 20:10 ` Junio C Hamano @ 2007-02-16 20:30 ` Santi Béjar 2007-02-16 21:14 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Santi Béjar @ 2007-02-16 20:30 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List On 2/16/07, Junio C Hamano <junkio@cox.net> wrote: > "Santi Béjar" <sbejar@gmail.com> writes: > > > In the past we've had problems when we have changed the merge logic > > (as shows 4363dfbe3). This patch makes the two process completely > > independent, and concentrate the merge logic in one place (leaving > > git-parse-remote.sh independent of the merge logic). > > But that is a solved problem, isn't it? What else does it > solve? The justification for moving around the logic could be > something like "these three patches do not do that themselves, > but it opens a door for further work such as ...", but without > something concrete in "..." part, your response makes the patch > look mostly needless code churn. > > I was hoping to hear something like "now git-fetch has to do > much less than before, eventual C-rewrite of the command, which > can borrow some code already written for 'pu' branch, will > become much easier" ;-). This sounds like a good justification, also :-) Santi ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH/RFC 0/3] Split fetch and merge logic 2007-02-16 20:30 ` Santi Béjar @ 2007-02-16 21:14 ` Junio C Hamano 2007-02-19 9:47 ` Santi Béjar 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2007-02-16 21:14 UTC (permalink / raw) To: Santi Béjar; +Cc: Git Mailing List "Santi Béjar" <sbejar@gmail.com> writes: > This sounds like a good justification, also :-) What I said was an example. I do not think your code churning actually would make it easier. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH/RFC 0/3] Split fetch and merge logic 2007-02-16 21:14 ` Junio C Hamano @ 2007-02-19 9:47 ` Santi Béjar 0 siblings, 0 replies; 14+ messages in thread From: Santi Béjar @ 2007-02-19 9:47 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List On 2/16/07, Junio C Hamano <junkio@cox.net> wrote: > "Santi Béjar" <sbejar@gmail.com> writes: > > > In the past we've had problems when we have changed the merge logic > > (as shows 4363dfbe3). This patch makes the two process completely > > independent, and concentrate the merge logic in one place (leaving > > git-parse-remote.sh independent of the merge logic). > > But that is a solved problem, isn't it? What else does it > solve? The justification for moving around the logic could be > something like "these three patches do not do that themselves, > but it opens a door for further work such as ...", but without > something concrete in "..." part, your response makes the patch > look mostly needless code churn. > > I was hoping to hear something like "now git-fetch has to do > much less than before, eventual C-rewrite of the command, which > can borrow some code already written for 'pu' branch, will > become much easier" ;-). On 2/16/07, Junio C Hamano <junkio@cox.net> wrote: > "Santi Béjar" <sbejar@gmail.com> writes: > > > This sounds like a good justification, also :-) > > What I said was an example. I do not think your code churning > actually would make it easier. > > What I said was an example of why having the two logic intermixed can be problematic. Anyway, basically: * It is a cleanup * It makes the code cleaner * It makes the code easier to maintain * It makes the future changes easier * It makes git-parse-remote and almost all git-fetch independent of the merge logic * The merge logic is in a few lines * Do one thing and do it well Regards, Santi ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <87zm7eo78x.fsf@gmail.com>]
* Re: [PATCH/RFC 1/3] t/t5515-fetch-merge-logic.sh: Added tests for the merge login in git-fetch [not found] ` <87zm7eo78x.fsf@gmail.com> @ 2007-02-16 8:23 ` Santi Béjar 0 siblings, 0 replies; 14+ messages in thread From: Santi Béjar @ 2007-02-16 8:23 UTC (permalink / raw) To: Git Mailing List [-- Attachment #1: Type: text/plain, Size: 3704 bytes --] Hi *, I think it didn't get to the list due to the size limit. So I send it as a gziped attatchment, with the test inline. Regards, Santi --- t/t5515-fetch-merge-logic.sh | 124 ++++++++++++++++++++ diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh new file mode 100755 index 0000000..8d8a71f --- /dev/null +++ b/t/t5515-fetch-merge-logic.sh @@ -0,0 +1,124 @@ +#!/bin/sh +# +# Copyright (c) 2007 Santi Béjar, based on t4013 by Junio C Hamano +# +# + +test_description='Merge logic in fetch' + +. ./test-lib.sh + +LF=' +' + +test_expect_success setup ' + GIT_AUTHOR_DATE="2006-06-26 00:00:00 +0000" && + GIT_COMMITTER_DATE="2006-06-26 00:00:00 +0000" && + export GIT_AUTHOR_DATE GIT_COMMITTER_DATE && + + echo >file original && + git add file && + git commit -a -m One && + git branch one && + + echo two >> file && + git commit -a -m Two && + git branch two && + + echo three >> file && + git commit -a -m Three && + git branch three && + + echo master >> file && + git commit -a -m Master + + git checkout three + + git clone . cloned && + cd cloned && + + git config remote.config-explicit.url ../.git/ && + git config remote.config-explicit.fetch refs/heads/master:remotes/rem/master && + git config --add remote.config-explicit.fetch refs/heads/one:remotes/rem/one && + git config --add remote.config-explicit.fetch two:remotes/rem/two && + git config --add remote.config-explicit.fetch refs/heads/three:remotes/rem/three && + + git config remote.config-glob.url ../.git/ && + git config remote.config-glob.fetch refs/heads/*:refs/remotes/rem/* && + + mkdir -p .git/remotes && + { + echo "URL: ../.git/" + echo "Pull: refs/heads/master:remotes/rem/master" + echo "Pull: refs/heads/one:remotes/rem/one" + echo "Pull: two:remotes/rem/two" + echo "Pull: refs/heads/three:remotes/rem/three" + } >.git/remotes/remote-explicit && + + { + echo "URL: ../.git/" + echo "Pull: refs/heads/*:refs/remotes/rem/*" + } >.git/remotes/remote-glob && + + mkdir -p .git/branches && + echo "../.git" > .git/branches/branches-default && + echo "../.git#one" > .git/branches/branches-one && + + for remote in config-explicit config-glob remote-explicit remote-glob \ + branches-default branches-one ; do + git config branch.br-$remote.remote $remote && + git config branch.br-$remote-merge.remote $remote && + git config branch.br-$remote-merge.merge refs/heads/three && + git config branch.br-$remote-octopus.remote $remote && + git config branch.br-$remote-octopus.merge refs/heads/one && + git config --add branch.br-$remote-octopus.merge two && + git config --add branch.br-$remote-octopus.merge remotes/rem/three && + branches="$branches br-$remote br-$remote-merge br-$remote-octopus" + done +' + +for branch in br-unconfig $branches ; do + echo $branch + for remote in config-explicit config-glob remote-explicit remote-glob \ + branches-default branches-one ../.git + do +cat <<EOF +$branch $remote +$branch $remote one +$branch $remote one two +EOF + done +done > tests + +while read cmd +do + case "$cmd" in + '' | '#'*) continue ;; + esac + test=`echo "$cmd" | sed -e 's|[/ ][/ ]*|_|g'` + cnt=`expr $test_count + 1` + pfx=`printf "%04d" $cnt` + expect="../../t5515/fetch.$test" + actual="$pfx-fetch.$test" + + test_expect_success "$cmd" ' + { + echo "# $cmd" + set x $cmd; shift + git symbolic-ref HEAD refs/heads/$1 ; shift + git fetch "$@" >/dev/null + cut -f -2 .git/FETCH_HEAD + } >"$actual" && + if test -f "$expect" + then + diff -u "$expect" "$actual" && + rm -f "$actual" + else + # this is to help developing new tests. + cp "$actual" "$expect" + false + fi + ' +done < tests + +test_done [-- Attachment #2: 0001-t-t5515-fetch-merge-logic.sh-Added-tests-for-the-merge-login-in-git-fetch.patch.gz --] [-- Type: application/x-gzip, Size: 13870 bytes --] ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2007-02-20 11:22 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-16 8:06 [PATCH/RFC 0/3] Split fetch and merge logic Santi Béjar
2007-02-16 8:09 ` [PATCH/RFC 2/3] git-fetch: " Santi Béjar
2007-02-19 20:44 ` Junio C Hamano
2007-02-19 22:13 ` Santi Béjar
2007-02-19 23:27 ` Junio C Hamano
2007-02-20 11:21 ` Santi Béjar
2007-02-16 8:10 ` [PATCH/RFC 3/3] t/t5515: fixes for the separate " Santi Béjar
2007-02-16 8:22 ` [PATCH/RFC 0/3] Split " Junio C Hamano
2007-02-16 8:40 ` Santi Béjar
2007-02-16 20:10 ` Junio C Hamano
2007-02-16 20:30 ` Santi Béjar
2007-02-16 21:14 ` Junio C Hamano
2007-02-19 9:47 ` Santi Béjar
[not found] ` <87zm7eo78x.fsf@gmail.com>
2007-02-16 8:23 ` [PATCH/RFC 1/3] t/t5515-fetch-merge-logic.sh: Added tests for the merge login in git-fetch Santi Béjar
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.