* [PATCHv3] Make git-clone respect branch.autosetuprebase @ 2009-03-03 20:23 Pat Notz 2009-03-04 6:29 ` Junio C Hamano 0 siblings, 1 reply; 4+ messages in thread From: Pat Notz @ 2009-03-03 20:23 UTC (permalink / raw) To: git; +Cc: Pat Notz When git-clone creates an initial branch it was not checking the branch.autosetuprebase configuration option (which may exist in ~/.gitconfig). Added function to branch.c to handle autorebase configuration setup. Signed-off-by: Pat Notz <pknotz@sandia.gov> --- Sorry, the PATCHv2 email had a stray "From:..." line. branch.c | 23 ++++++++++++++++------- branch.h | 9 +++++++++ builtin-clone.c | 3 +++ t/t5601-clone.sh | 14 ++++++++++++++ 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/branch.c b/branch.c index 1f00e44..ee8b69d 100644 --- a/branch.c +++ b/branch.c @@ -32,21 +32,33 @@ static int find_tracked_branch(struct remote *remote, void *priv) return 0; } -static int should_setup_rebase(const struct tracking *tracking) +static int should_setup_rebase(const char * remote) { switch (autorebase) { case AUTOREBASE_NEVER: return 0; case AUTOREBASE_LOCAL: - return tracking->remote == NULL; + return remote == NULL; case AUTOREBASE_REMOTE: - return tracking->remote != NULL; + return remote != NULL; case AUTOREBASE_ALWAYS: return 1; } return 0; } +int setup_autorebase(const char *local, const char *remote_tracked_branch) +{ + if (should_setup_rebase(remote_tracked_branch)) { + struct strbuf key = STRBUF_INIT; + strbuf_addf(&key, "branch.%s.rebase", local); + git_config_set(key.buf, "true"); + strbuf_release(&key); + return 1; + } + return 0; +} + /* * This is called when new_ref is branched off of orig_ref, and tries * to infer the settings for branch.<new_ref>.{remote,merge} from the @@ -86,11 +98,8 @@ static int setup_tracking(const char *new_ref, const char *orig_ref, git_config_set(key, tracking.src ? tracking.src : orig_ref); printf("Branch %s set up to track %s branch %s.\n", new_ref, tracking.remote ? "remote" : "local", orig_ref); - if (should_setup_rebase(&tracking)) { - sprintf(key, "branch.%s.rebase", new_ref); - git_config_set(key, "true"); + if (setup_autorebase(new_ref, tracking.remote)) printf("This branch will rebase on pull.\n"); - } free(tracking.src); return 0; diff --git a/branch.h b/branch.h index 9f0c2a2..b332c09 100644 --- a/branch.h +++ b/branch.h @@ -21,4 +21,13 @@ void create_branch(const char *head, const char *name, const char *start_name, */ void remove_branch_state(void); +/* + * Used when creating a new branch to properly setup the autorebase flag + * in the git-config file. If the new branch ("local") is created from + * a remote branch then remote_tracked_branch should be non-NULL. If + * the new branch is being created from another local branch then + * remote_tracked_branch should be NULL. + */ +int setup_autorebase(const char *local, const char *remote_tracked_branch); + #endif diff --git a/builtin-clone.c b/builtin-clone.c index c338910..587c834 100644 --- a/builtin-clone.c +++ b/builtin-clone.c @@ -17,6 +17,7 @@ #include "unpack-trees.h" #include "transport.h" #include "strbuf.h" +#include "branch.h" #include "dir.h" #include "pack-refs.h" #include "sigchain.h" @@ -360,6 +361,8 @@ static void install_branch_config(const char *local, strbuf_reset(&key); strbuf_addf(&key, "branch.%s.merge", local); git_config_set(key.buf, remote); + if (setup_autorebase(local, remote)) + printf("Default branch '%s' will rebase on pull.\n", local); strbuf_release(&key); } diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 44793f2..0f8b43c 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -159,4 +159,18 @@ test_expect_success 'clone a void' ' test_cmp target-6/.git/config target-7/.git/config ' +test_expect_success 'clone respects global branch.autosetuprebase' ' + HOME="`pwd`" && + export HOME && + test_config="$HOME"/.gitconfig && + unset GIT_CONFIG_NOGLOBAL && + git config -f "$test_config" branch.autosetuprebase remote && + rm -fr dst && + git clone src dst && + cd dst && + expected="ztrue" && + actual="z$(git config branch.master.rebase)" && + test $expected = $actual +' + test_done -- 1.6.1.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCHv3] Make git-clone respect branch.autosetuprebase 2009-03-03 20:23 [PATCHv3] Make git-clone respect branch.autosetuprebase Pat Notz @ 2009-03-04 6:29 ` Junio C Hamano 2009-03-06 2:58 ` [PATCHv4] " Pat Notz 0 siblings, 1 reply; 4+ messages in thread From: Junio C Hamano @ 2009-03-04 6:29 UTC (permalink / raw) To: Pat Notz; +Cc: git "Pat Notz" <pknotz@sandia.gov> writes: > When git-clone creates an initial branch it was not > checking the branch.autosetuprebase configuration > option (which may exist in ~/.gitconfig). > > Added function to branch.c to handle autorebase > configuration setup. > > Signed-off-by: Pat Notz <pknotz@sandia.gov> Thanks. Your patch still has two codepaths that set the branch.*.merge and other information and they can diverge with later changes. You can refactor them even more to avoid that risk, like the attached patch. The test is taken from your patch but runs inside a subshell, so that later tests others may add to the script will be safer from the environment variable settings and chdir this test does. -- >8 -- When git-clone creates an initial branch it was not checking the branch.autosetuprebase configuration option (which may exist in ~/.gitconfig). Refactor the code used by "git branch" to create a new branch, and use it instead of the insufficiently duplicated code in builtin-clone. --- branch.c | 49 +++++++++++++++++++++++++++++++++---------------- branch.h | 7 +++++++ builtin-clone.c | 18 +++--------------- t/t5601-clone.sh | 15 +++++++++++++++ 4 files changed, 58 insertions(+), 31 deletions(-) diff --git a/branch.c b/branch.c index 1f00e44..d20fb04 100644 --- a/branch.c +++ b/branch.c @@ -32,21 +32,48 @@ static int find_tracked_branch(struct remote *remote, void *priv) return 0; } -static int should_setup_rebase(const struct tracking *tracking) +static int should_setup_rebase(const char *origin) { switch (autorebase) { case AUTOREBASE_NEVER: return 0; case AUTOREBASE_LOCAL: - return tracking->remote == NULL; + return origin == NULL; case AUTOREBASE_REMOTE: - return tracking->remote != NULL; + return origin != NULL; case AUTOREBASE_ALWAYS: return 1; } return 0; } +void install_branch_config(int flag, const char *local, const char *origin, const char *remote) +{ + struct strbuf key = STRBUF_INIT; + int rebasing = should_setup_rebase(origin); + + strbuf_addf(&key, "branch.%s.remote", local); + git_config_set(key.buf, origin ? origin : "."); + + strbuf_reset(&key); + strbuf_addf(&key, "branch.%s.merge", local); + git_config_set(key.buf, remote); + + if (rebasing) { + strbuf_reset(&key); + strbuf_addf(&key, "branch.%s.rebase", local); + git_config_set(key.buf, "true"); + } + + if (flag & BRANCH_CONFIG_VERBOSE) + printf("Branch %s set up to track %s branch %s %s.\n", + local, + origin ? "remote" : "local", + remote, + rebasing ? "by rebasing" : "by merging"); + strbuf_release(&key); +} + /* * This is called when new_ref is branched off of orig_ref, and tries * to infer the settings for branch.<new_ref>.{remote,merge} from the @@ -55,7 +82,6 @@ static int should_setup_rebase(const struct tracking *tracking) static int setup_tracking(const char *new_ref, const char *orig_ref, enum branch_track track) { - char key[1024]; struct tracking tracking; if (strlen(new_ref) > 1024 - 7 - 7 - 1) @@ -80,19 +106,10 @@ static int setup_tracking(const char *new_ref, const char *orig_ref, return error("Not tracking: ambiguous information for ref %s", orig_ref); - sprintf(key, "branch.%s.remote", new_ref); - git_config_set(key, tracking.remote ? tracking.remote : "."); - sprintf(key, "branch.%s.merge", new_ref); - git_config_set(key, tracking.src ? tracking.src : orig_ref); - printf("Branch %s set up to track %s branch %s.\n", new_ref, - tracking.remote ? "remote" : "local", orig_ref); - if (should_setup_rebase(&tracking)) { - sprintf(key, "branch.%s.rebase", new_ref); - git_config_set(key, "true"); - printf("This branch will rebase on pull.\n"); - } - free(tracking.src); + install_branch_config(BRANCH_CONFIG_VERBOSE, new_ref, tracking.remote, + tracking.src ? tracking.src : orig_ref); + free(tracking.src); return 0; } diff --git a/branch.h b/branch.h index 9f0c2a2..eed817a 100644 --- a/branch.h +++ b/branch.h @@ -21,4 +21,11 @@ void create_branch(const char *head, const char *name, const char *start_name, */ void remove_branch_state(void); +/* + * Configure local branch "local" to merge remote branch "remote" + * taken from origin "origin". + */ +#define BRANCH_CONFIG_VERBOSE 01 +extern void install_branch_config(int flag, const char *local, const char *origin, const char *remote); + #endif diff --git a/builtin-clone.c b/builtin-clone.c index c338910..a5f000a 100644 --- a/builtin-clone.c +++ b/builtin-clone.c @@ -20,6 +20,7 @@ #include "dir.h" #include "pack-refs.h" #include "sigchain.h" +#include "branch.h" /* * Overall FIXMEs: @@ -350,19 +351,6 @@ static struct ref *write_remote_refs(const struct ref *refs, return local_refs; } -static void install_branch_config(const char *local, - const char *origin, - const char *remote) -{ - struct strbuf key = STRBUF_INIT; - strbuf_addf(&key, "branch.%s.remote", local); - git_config_set(key.buf, origin); - strbuf_reset(&key); - strbuf_addf(&key, "branch.%s.merge", local); - git_config_set(key.buf, remote); - strbuf_release(&key); -} - int cmd_clone(int argc, const char **argv, const char *prefix) { int use_local_hardlinks = 1; @@ -553,7 +541,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) remote_head = NULL; option_no_checkout = 1; if (!option_bare) - install_branch_config("master", option_origin, + install_branch_config(0, "master", option_origin, "refs/heads/master"); } @@ -583,7 +571,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) head_points_at->peer_ref->name, reflog_msg.buf); - install_branch_config(head, option_origin, + install_branch_config(0, head, option_origin, head_points_at->name); } } else if (remote_head) { diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 44793f2..2335d8b 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -159,4 +159,19 @@ test_expect_success 'clone a void' ' test_cmp target-6/.git/config target-7/.git/config ' +test_expect_success 'clone respects global branch.autosetuprebase' ' + ( + HOME=$(pwd) && + export HOME && + test_config="$HOME/.gitconfig" && + unset GIT_CONFIG_NOGLOBAL && + git config -f "$test_config" branch.autosetuprebase remote && + rm -fr dst && + git clone src dst && + cd dst && + actual="z$(git config branch.master.rebase)" && + test ztrue = $actual + ) +' + test_done ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCHv4] Make git-clone respect branch.autosetuprebase 2009-03-04 6:29 ` Junio C Hamano @ 2009-03-06 2:58 ` Pat Notz 2009-03-06 5:29 ` Junio C Hamano 0 siblings, 1 reply; 4+ messages in thread From: Pat Notz @ 2009-03-06 2:58 UTC (permalink / raw) To: git; +Cc: Pat Notz When git-clone creates an initial branch it was not checking the branch.autosetuprebase configuration option (which may exist in ~/.gitconfig). Create a single function for installing a branch into the config file. Both 'clone' and 'branch' now use the same function for installing branches. Refactored as suggested by Junio C. Hamano. Signed-off-by: Pat Notz <pknotz@sandia.gov> --- branch.c | 54 ++++++++++++++++++++++++++++++++++++++---------------- branch.h | 11 +++++++++++ builtin-clone.c | 24 ++++++++---------------- t/t5601-clone.sh | 15 +++++++++++++++ 4 files changed, 72 insertions(+), 32 deletions(-) diff --git a/branch.c b/branch.c index 1f00e44..332223b 100644 --- a/branch.c +++ b/branch.c @@ -32,21 +32,52 @@ static int find_tracked_branch(struct remote *remote, void *priv) return 0; } -static int should_setup_rebase(const struct tracking *tracking) +static int should_setup_rebase(const char * origin) { switch (autorebase) { case AUTOREBASE_NEVER: return 0; case AUTOREBASE_LOCAL: - return tracking->remote == NULL; + return origin == NULL; case AUTOREBASE_REMOTE: - return tracking->remote != NULL; + return origin != NULL; case AUTOREBASE_ALWAYS: return 1; } return 0; } +void install_branch_config(int verbose_flag, + const char *local, + const char *origin, + const char *remote) +{ + struct strbuf key = STRBUF_INIT; + int rebasing = should_setup_rebase(origin); + + strbuf_addf(&key, "branch.%s.remote", local); + git_config_set(key.buf, origin ? origin : "."); + + strbuf_reset(&key); + strbuf_addf(&key, "branch.%s.merge", local); + git_config_set(key.buf, remote); + + if (rebasing) { + strbuf_reset(&key); + strbuf_addf(&key, "branch.%s.rebase", local); + git_config_set(key.buf, "true"); + } + + if (verbose_flag & BRANCH_CONFIG_VERBOSE) + printf("Branch %s set up to track %s branch %s %s.\n", + local, + origin ? "remote" : "local", + remote, + rebasing ? "by rebasing" : "by merging"); + strbuf_release(&key); +} + + /* * This is called when new_ref is branched off of orig_ref, and tries * to infer the settings for branch.<new_ref>.{remote,merge} from the @@ -55,7 +86,6 @@ static int should_setup_rebase(const struct tracking *tracking) static int setup_tracking(const char *new_ref, const char *orig_ref, enum branch_track track) { - char key[1024]; struct tracking tracking; if (strlen(new_ref) > 1024 - 7 - 7 - 1) @@ -79,18 +109,10 @@ static int setup_tracking(const char *new_ref, const char *orig_ref, if (tracking.matches > 1) return error("Not tracking: ambiguous information for ref %s", orig_ref); - - sprintf(key, "branch.%s.remote", new_ref); - git_config_set(key, tracking.remote ? tracking.remote : "."); - sprintf(key, "branch.%s.merge", new_ref); - git_config_set(key, tracking.src ? tracking.src : orig_ref); - printf("Branch %s set up to track %s branch %s.\n", new_ref, - tracking.remote ? "remote" : "local", orig_ref); - if (should_setup_rebase(&tracking)) { - sprintf(key, "branch.%s.rebase", new_ref); - git_config_set(key, "true"); - printf("This branch will rebase on pull.\n"); - } + install_branch_config(BRANCH_CONFIG_VERBOSE, + new_ref, + tracking.remote, + tracking.src ? tracking.src : orig_ref); free(tracking.src); return 0; diff --git a/branch.h b/branch.h index 9f0c2a2..9f7fdb0 100644 --- a/branch.h +++ b/branch.h @@ -21,4 +21,15 @@ void create_branch(const char *head, const char *name, const char *start_name, */ void remove_branch_state(void); +/* + * Configure local branch "local" to merge remote branch "remote" + * taken from origin "origin". + */ +#define BRANCH_CONFIG_QUIET 00 +#define BRANCH_CONFIG_VERBOSE 01 +void install_branch_config(int verbose_flag, + const char *local, + const char *origin, + const char *remote); + #endif diff --git a/builtin-clone.c b/builtin-clone.c index c338910..e1abf83 100644 --- a/builtin-clone.c +++ b/builtin-clone.c @@ -17,6 +17,7 @@ #include "unpack-trees.h" #include "transport.h" #include "strbuf.h" +#include "branch.h" #include "dir.h" #include "pack-refs.h" #include "sigchain.h" @@ -350,19 +351,6 @@ static struct ref *write_remote_refs(const struct ref *refs, return local_refs; } -static void install_branch_config(const char *local, - const char *origin, - const char *remote) -{ - struct strbuf key = STRBUF_INIT; - strbuf_addf(&key, "branch.%s.remote", local); - git_config_set(key.buf, origin); - strbuf_reset(&key); - strbuf_addf(&key, "branch.%s.merge", local); - git_config_set(key.buf, remote); - strbuf_release(&key); -} - int cmd_clone(int argc, const char **argv, const char *prefix) { int use_local_hardlinks = 1; @@ -553,8 +541,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix) remote_head = NULL; option_no_checkout = 1; if (!option_bare) - install_branch_config("master", option_origin, - "refs/heads/master"); + install_branch_config(BRANCH_CONFIG_QUIET, + "master", + option_origin, + "refs/heads/master"); } if (head_points_at) { @@ -583,7 +573,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix) head_points_at->peer_ref->name, reflog_msg.buf); - install_branch_config(head, option_origin, + install_branch_config(BRANCH_CONFIG_QUIET, + head, + option_origin, head_points_at->name); } } else if (remote_head) { diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 44793f2..fa10573 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -159,4 +159,19 @@ test_expect_success 'clone a void' ' test_cmp target-6/.git/config target-7/.git/config ' +test_expect_success 'clone respects global branch.autosetuprebase' ' + ( + HOME="`pwd`" && + export HOME && + test_config="$HOME"/.gitconfig && + unset GIT_CONFIG_NOGLOBAL && + git config -f "$test_config" branch.autosetuprebase remote && + rm -fr dst && + git clone src dst && + cd dst && + actual="z$(git config branch.master.rebase)" && + test ztrue = $actual + ) +' + test_done -- 1.6.1.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCHv4] Make git-clone respect branch.autosetuprebase 2009-03-06 2:58 ` [PATCHv4] " Pat Notz @ 2009-03-06 5:29 ` Junio C Hamano 0 siblings, 0 replies; 4+ messages in thread From: Junio C Hamano @ 2009-03-06 5:29 UTC (permalink / raw) To: Pat Notz; +Cc: git "Pat Notz" <pknotz@sandia.gov> writes: > Signed-off-by: Pat Notz <pknotz@sandia.gov> Thanks. I do not think there is substantial difference from the one I queued in 'pu', so I'd keep that one instead, but here are a few comments for future reference. > diff --git a/branch.c b/branch.c > index 1f00e44..332223b 100644 > --- a/branch.c > +++ b/branch.c > @@ -32,21 +32,52 @@ static int find_tracked_branch(struct remote *remote, void *priv) > return 0; > } > > -static int should_setup_rebase(const struct tracking *tracking) > +static int should_setup_rebase(const char * origin) "const char *origin"; > +void install_branch_config(int verbose_flag, > + const char *local, > + const char *origin, > + const char *remote) > +{ I called the parameter "flag" not "verbose" very much on purpose; we can introduce flags that control aspects other than verbosity if we wanted to later. Renaming it to verbose_flag defeats the extensibility. > diff --git a/branch.h b/branch.h > index 9f0c2a2..9f7fdb0 100644 > --- a/branch.h > +++ b/branch.h > @@ -21,4 +21,15 @@ void create_branch(const char *head, const char *name, const char *start_name, > */ > void remove_branch_state(void); > > +/* > + * Configure local branch "local" to merge remote branch "remote" > + * taken from origin "origin". > + */ > +#define BRANCH_CONFIG_QUIET 00 This QUIET is also against the extensible design, as later flags may mean things unrelated to verbosity. With bitfield "flags" parameter, an unset bit simply means "do not trigger this feature" and it is customary to pass 0 to such a function when the caller does not want anything special. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-03-06 5:31 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-03-03 20:23 [PATCHv3] Make git-clone respect branch.autosetuprebase Pat Notz 2009-03-04 6:29 ` Junio C Hamano 2009-03-06 2:58 ` [PATCHv4] " Pat Notz 2009-03-06 5:29 ` Junio C Hamano
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).