Re: [PATCH v2 3/8] refs: factor update_ref steps into helpers
Brad King brad.k...@kitware.com writes:
Factor the lock and write steps and error handling into helper functions
update_ref_lock and update_ref_write to allow later use elsewhere.
Expose lock_any_ref_for_update's type_p to update_ref_lock callers.
Signed-off-by: Brad King brad.k...@kitware.com
---
refs.c | 28 +++-
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index c69fd68..2e755b4 100644
--- a/refs.c
+++ b/refs.c
@@ -3170,12 +3170,13 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
return retval;
}
-int update_ref(const char *action, const char *refname,
- const unsigned char *sha1, const unsigned char *oldval,
- int flags, enum action_on_err onerr)
+static struct ref_lock *update_ref_lock(const char *refname,
+ const unsigned char *oldval,
+ int flags, int *type_p,
+ enum action_on_err onerr)
{
static struct ref_lock *lock;
Not the fault of this patch, as the original update_ref() had it
this way, but it is not necessary to keep the value of this variable
across invocations. Let's drop static from here, and also the
corresponding variable in the new update_ref().
Will locally tweak while queuing.
- lock = lock_any_ref_for_update(refname, oldval, flags, NULL);
+ lock = lock_any_ref_for_update(refname, oldval, flags, type_p);
if (!lock) {
const char *str = Cannot lock the ref '%s'.;
switch (onerr) {
@@ -3183,8 +3184,14 @@ int update_ref(const char *action, const char *refname,
case DIE_ON_ERR: die(str, refname); break;
case QUIET_ON_ERR: break;
}
- return 1;
}
+ return lock;
+}
+
+static int update_ref_write(const char *action, const char *refname,
+ const unsigned char *sha1, struct ref_lock *lock,
+ enum action_on_err onerr)
+{
if (write_ref_sha1(lock, sha1, action) 0) {
const char *str = Cannot update the ref '%s'.;
switch (onerr) {
@@ -3197,6 +3204,17 @@ int update_ref(const char *action, const char *refname,
return 0;
}
+int update_ref(const char *action, const char *refname,
+const unsigned char *sha1, const unsigned char *oldval,
+int flags, enum action_on_err onerr)
+{
+ static struct ref_lock *lock;
+ lock = update_ref_lock(refname, oldval, flags, 0, onerr);
+ if (!lock)
+ return 1;
+ return update_ref_write(action, refname, sha1, lock, onerr);
+}
+
struct ref *find_ref_by_name(const struct ref *list, const char *name)
{
for ( ; list; list = list-next)
--
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 6/8] refs: add update_refs for multiple simultaneous updates
Brad King brad.k...@kitware.com writes:
Add 'struct ref_update' to encode the information needed to update or
delete a ref (name, new sha1, optional old sha1, no-deref flag). Add
function 'update_refs' accepting an array of updates to perform. First
sort the input array to order locks consistently everywhere and reject
multiple updates to the same ref. Then acquire locks on all refs with
verified old values. Then update or delete all refs accordingly. Fail
if any one lock cannot be obtained or any one old value does not match.
OK. The code releases the locks it acquired so far when it fails,
which is good.
Though the refs themeselves cannot be modified together in a single
themselves.
atomic transaction, this function does enable some useful semantics.
For example, a caller may create a new branch starting from the head of
another branch and rewind the original branch at the same time. This
transfers ownership of commits between branches without risk of losing
commits added to the original branch by a concurrent process, or risk of
a concurrent process creating the new branch first.
+static int ref_update_compare(const void *r1, const void *r2)
+{
+ struct ref_update *u1 = (struct ref_update *)(r1);
+ struct ref_update *u2 = (struct ref_update *)(r2);
+ int ret;
Let's have a blank line between the end of decls and the beginning
of the body here.
+ ret = strcmp(u1-ref_name, u2-ref_name);
+ if (ret)
+ return ret;
+ ret = hashcmp(u1-new_sha1, u2-new_sha1);
+ if (ret)
+ return ret;
+ ret = hashcmp(u1-old_sha1, u2-old_sha1);
+ if (ret)
+ return ret;
+ ret = u1-flags - u2-flags;
+ if (ret)
+ return ret;
+ return u1-have_old - u2-have_old;
+}
I notice that we are using an array of structures and letting qsort
swap 50~64 bytes of data, instead of sorting an array of pointers,
each element of which points at a structure. This may not matter
unless we are asked to update thousands at once, so I think it is OK
for now.
+static int ref_update_reject_duplicates(struct ref_update *updates, int n,
+ enum action_on_err onerr)
+{
+ int i;
+ for (i = 1; i n; ++i)
+ if (!strcmp(updates[i - 1].ref_name, updates[i].ref_name))
+ break;
Optionally we could silently dedup multiple identical updates and
not fail it in ref-update-reject-duplicates. But that does not have
to be done until we find people's script would benefit from such a
nicety.
By the way, unless there is a strong reason not to do so,
post-increment i++ (and pre-decrement --i, if you use it) is the
norm around here. Especially in places like the third part of a
for(;;) loop where people are used to see i++, breaking the idiom
makes readers wonder if there is something else going on.
+ /* Perform updates first so live commits remain referenced: */
+ for (i = 0; i n; ++i)
+ if (!is_null_sha1(updates[i].new_sha1)) {
+ ret |= update_ref_write(action,
+ updates[i].ref_name,
+ updates[i].new_sha1,
+ locks[i], onerr);
+ locks[i] = 0; /* freed by update_ref_write */
I think what is assigned here is a NULL pointer.
Will locally tweak while queuing. Thanks.
--
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 2/2] fast-export: refactor get_tags_and_duplicates()
Split into a separate helper function get_commit() so that the part that
finds the relevant commit, and the part that does something with it
(handle tag object, etc.) are in different places.
No functional changes.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/fast-export.c | 68 ---
1 file changed, 38 insertions(+), 30 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 957392c..03e1090 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -485,9 +485,32 @@ static void handle_tag(const char *name, struct tag *tag)
(int)message_size, (int)message_size, message ? message : );
}
+static struct commit *get_commit(struct rev_cmdline_entry *e, char *full_name)
+{
+ switch (e-item-type) {
+ case OBJ_COMMIT:
+ return (struct commit *)e-item;
+ case OBJ_TAG: {
+ struct tag *tag = (struct tag *)e-item;
+
+ /* handle nested tags */
+ while (tag tag-object.type == OBJ_TAG) {
+ parse_object(tag-object.sha1);
+ string_list_append(extra_refs, full_name)-util = tag;
+ tag = (struct tag *)tag-tagged;
+ }
+ if (!tag)
+ die(Tag %s points nowhere?, e-name);
+ return (struct commit *)tag;
+ break;
+ }
+ default:
+ return NULL;
+ }
+}
+
static void get_tags_and_duplicates(struct rev_cmdline_info *info)
{
- struct tag *tag;
int i;
for (i = 0; i info-nr; i++) {
@@ -502,41 +525,26 @@ static void get_tags_and_duplicates(struct
rev_cmdline_info *info)
if (dwim_ref(e-name, strlen(e-name), sha1, full_name) != 1)
continue;
- switch (e-item-type) {
- case OBJ_COMMIT:
- commit = (struct commit *)e-item;
- break;
- case OBJ_TAG:
- tag = (struct tag *)e-item;
-
- /* handle nested tags */
- while (tag tag-object.type == OBJ_TAG) {
- parse_object(tag-object.sha1);
- string_list_append(extra_refs,
full_name)-util = tag;
- tag = (struct tag *)tag-tagged;
- }
- if (!tag)
- die (Tag %s points nowhere?, e-name);
- switch(tag-object.type) {
- case OBJ_COMMIT:
- commit = (struct commit *)tag;
- break;
- case OBJ_BLOB:
- export_blob(tag-object.sha1);
- continue;
- default: /* OBJ_TAG (nested tags) is already handled */
- warning(Tag points to object of unexpected
type %s, skipping.,
- typename(tag-object.type));
- continue;
- }
- break;
- default:
+ commit = get_commit(e, full_name);
+ if (!commit) {
warning(%s: Unexpected object of type %s, skipping.,
e-name,
typename(e-item-type));
continue;
}
+ switch(commit-object.type) {
+ case OBJ_COMMIT:
+ break;
+ case OBJ_BLOB:
+ export_blob(commit-object.sha1);
+ continue;
+ default: /* OBJ_TAG (nested tags) is already handled */
+ warning(Tag points to object of unexpected type %s,
skipping.,
+ typename(commit-object.type));
+ continue;
+ }
+
/*
* This ref will not be updated through a commit, lets make
* sure it gets properly updated eventually.
--
1.8.4-337-g7358a66-dirty
--
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 0/2] fast-export: simplification
Hi, No functional changes, but get_tags_and_duplicates() is quite complex as it is, and can be simplified by spliting code into a separate function. Felipe Contreras (2): fast-export: make extra_refs global fast-export: refactor get_tags_and_duplicates() builtin/fast-export.c | 87 --- 1 file changed, 47 insertions(+), 40 deletions(-) -- 1.8.4-337-g7358a66-dirty -- 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 1/2] fast-export: make extra_refs global
There's no need to pass it around everywhere. This would make easier
further refactoring that makes use of this variable.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/fast-export.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 8e19058..957392c 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -30,6 +30,7 @@ static int fake_missing_tagger;
static int use_done_feature;
static int no_data;
static int full_tree;
+static struct string_list extra_refs = STRING_LIST_INIT_NODUP;
static int parse_opt_signed_tag_mode(const struct option *opt,
const char *arg, int unset)
@@ -484,8 +485,7 @@ static void handle_tag(const char *name, struct tag *tag)
(int)message_size, (int)message_size, message ? message : );
}
-static void get_tags_and_duplicates(struct rev_cmdline_info *info,
- struct string_list *extra_refs)
+static void get_tags_and_duplicates(struct rev_cmdline_info *info)
{
struct tag *tag;
int i;
@@ -512,7 +512,7 @@ static void get_tags_and_duplicates(struct rev_cmdline_info
*info,
/* handle nested tags */
while (tag tag-object.type == OBJ_TAG) {
parse_object(tag-object.sha1);
- string_list_append(extra_refs, full_name)-util
= tag;
+ string_list_append(extra_refs,
full_name)-util = tag;
tag = (struct tag *)tag-tagged;
}
if (!tag)
@@ -542,20 +542,20 @@ static void get_tags_and_duplicates(struct
rev_cmdline_info *info,
* sure it gets properly updated eventually.
*/
if (commit-util || commit-object.flags SHOWN)
- string_list_append(extra_refs, full_name)-util =
commit;
+ string_list_append(extra_refs, full_name)-util =
commit;
if (!commit-util)
commit-util = full_name;
}
}
-static void handle_tags_and_duplicates(struct string_list *extra_refs)
+static void handle_tags_and_duplicates(void)
{
struct commit *commit;
int i;
- for (i = extra_refs-nr - 1; i = 0; i--) {
- const char *name = extra_refs-items[i].string;
- struct object *object = extra_refs-items[i].util;
+ for (i = extra_refs.nr - 1; i = 0; i--) {
+ const char *name = extra_refs.items[i].string;
+ struct object *object = extra_refs.items[i].util;
switch (object-type) {
case OBJ_TAG:
handle_tag(name, (struct tag *)object);
@@ -657,7 +657,6 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
{
struct rev_info revs;
struct object_array commits = OBJECT_ARRAY_INIT;
- struct string_list extra_refs = STRING_LIST_INIT_NODUP;
struct commit *commit;
char *export_filename = NULL, *import_filename = NULL;
uint32_t lastimportid;
@@ -709,7 +708,7 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
if (import_filename revs.prune_data.nr)
full_tree = 1;
- get_tags_and_duplicates(revs.cmdline, extra_refs);
+ get_tags_and_duplicates(revs.cmdline);
if (prepare_revision_walk(revs))
die(revision walk setup failed);
@@ -725,7 +724,7 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
}
}
- handle_tags_and_duplicates(extra_refs);
+ handle_tags_and_duplicates();
if (export_filename lastimportid != last_idnum)
export_marks(export_filename);
--
1.8.4-337-g7358a66-dirty
--
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 v3 08/11] t6050-replace: check that -f option bypasses the type check
On Sat, Aug 31, 2013 at 3:12 PM, Christian Couder
chrisc...@tuxfamily.org wrote:
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
t/t6050-replace.sh | 6 ++
1 file changed, 6 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 05be228..0b07a0b 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,6 +276,12 @@ test_expect_success 'replaced and replacement objects
must be of the same type'
grep $BLOB. points to a replacement object of type .blob err
'
+test_expect_success '-f option bypasses the type check' '
+ git replace -f mytag $HASH1 2err
+ git replace -f HEAD^{tree} HEAD~1 2err
+ git replace -f HEAD^ $BLOB 2err
+'
Is there a non-obvious reason you are redirecting stderr to a file in
this test? Unlike the test added earlier, this one never consults the
error output. By dropping this apparently unnecessary redirection,
diagnosis of a regression potentially becomes simpler since any error
output from git-replace will become visible when the test is run
verbosely.
+
test_expect_success 'replace ref cleanup' '
test -n $(git replace)
git replace -d $(git replace)
--
1.8.4.rc1.31.g530f5ce.dirty
--
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 v3 11/11] t6050-replace: use some long option names
On Sat, Aug 31, 2013 at 3:12 PM, Christian Couder
chrisc...@tuxfamily.org wrote:
So that they are tested a litlle bit too.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
t/t6050-replace.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 0b07a0b..5dc26e8 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -122,9 +122,9 @@ test_expect_success 'git replace listing and deleting' '
test $HASH2 = $(git replace -l)
test $HASH2 = $(git replace)
aa=${HASH2%??}
- test $HASH2 = $(git replace -l $aa*)
+ test $HASH2 = $(git replace --list $aa*)
test_must_fail git replace -d $R
- test_must_fail git replace -d
+ test_must_fail git replace --delete
test_must_fail git replace -l -d $HASH2
Is this sort of change a good idea? A person reading the code, but not
familiar with this particular patch, might not understand the
seemingly random choice of short versus long option usage. Such a
person might be led to wonder if there is some subtle difference
between the short and long forms, and then unnecessarily spend time
digging into the code and documentation in an attempt to figure it
out. Alternately, someone reading the code might be led to assume that
the person who added the tests was being sloppy.
Hopefully, t0040-parse-options should already be proof enough that
long options are correctly handled, but if you really want to test
them here too, it seems like a separate test would be more appropriate
than randomly changing short form options to long in various tests.
git replace -d $HASH2
git show $HASH2 | grep A U Thor
@@ -147,7 +147,7 @@ test_expect_success 'git replace resolves sha1' '
git show $HASH2 | grep O Thor
test_must_fail git replace $HASH2 $R
git replace -f $HASH2 $R
- test_must_fail git replace -f
+ test_must_fail git replace --force
test $HASH2 = $(git replace)
'
@@ -278,7 +278,7 @@ test_expect_success 'replaced and replacement objects
must be of the same type'
test_expect_success '-f option bypasses the type check' '
git replace -f mytag $HASH1 2err
- git replace -f HEAD^{tree} HEAD~1 2err
+ git replace --force HEAD^{tree} HEAD~1 2err
git replace -f HEAD^ $BLOB 2err
'
--
1.8.4.rc1.31.g530f5ce.dirty
--
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 4/6] t: branch add publish branch tests
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t3200-branch.sh | 76 +++
1 file changed, 76 insertions(+)
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 44ec6a4..cd0b8e9 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -870,4 +870,80 @@ test_expect_success '--merged catches invalid object
names' '
test_must_fail git branch --merged
'
+test_expect_success '--set-publish-to fails on multiple branches' '
+ test_must_fail git branch --set-publish-to master a b c
+'
+
+test_expect_success '--set-publish-to fails on detached HEAD' '
+ test_when_finished git checkout master
+ git checkout master^{}
+ test_must_fail git branch --set-publish-to master
+'
+
+test_expect_success '--set-publish-to fails on a missing dst branch' '
+ test_must_fail git branch --set-publish-to master does-not-exist
+'
+
+test_expect_success '--set-publish-to fails on a missing src branch' '
+ test_must_fail git branch --set-publish-to does-not-exist master
+'
+
+test_expect_success '--set-publish-to fails on a non-ref' '
+ test_must_fail git branch --set-publish-to HEAD^{}
+'
+
+test_expect_success 'use --set-publish-to modify HEAD' '
+ git checkout master
+ test_config branch.master.pushremote foo
+ test_config branch.master.push foo
+ git branch -f test
+ git branch --set-publish-to test
+ test $(git config branch.master.pushremote) = .
+ test $(git config branch.master.push) = refs/heads/test
+'
+
+test_expect_success 'use --set-publish-to modify a particular branch' '
+ git branch -f test
+ git branch -f test2
+ git branch --set-publish-to test2 test
+ test $(git config branch.test.pushremote) = .
+ test $(git config branch.test.push) = refs/heads/test2
+'
+
+test_expect_success '--unset-publish should fail if given a non-existent
branch' '
+ test_must_fail git branch --unset-publish i-dont-exist
+'
+
+test_expect_success 'test --unset-publish on HEAD' '
+ git checkout master
+ git branch -f test
+ test_config branch.master.pushremote foo
+ test_config branch.master.push foo
+ git branch --set-publish-to test
+ git branch --unset-publish
+ test_must_fail git config branch.master.pushremote
+ test_must_fail git config branch.master.push
+ # fail for a branch without publish set
+ test_must_fail git branch --unset-publish
+'
+
+test_expect_success '--unset-publish should fail on multiple branches' '
+ test_must_fail git branch --unset-publish a b c
+'
+
+test_expect_success '--unset-publish should fail on detached HEAD' '
+ test_when_finished git checkout -
+ git checkout HEAD^{}
+ test_must_fail git branch --unset-publish
+'
+
+test_expect_success 'test --unset-publish on a particular branch' '
+ git branch -f test
+ git branch -f test2
+ git branch --set-publish-to test2 test
+ git branch --unset-publish test
+ test_must_fail git config branch.test2.pushremote
+ test_must_fail git config branch.test2.push
+'
+
test_done
--
1.8.4-337-g7358a66-dirty
--
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 6/6] branch: display publish branch
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/branch.c | 45 -
1 file changed, 40 insertions(+), 5 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 48af999..47644ad 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -42,6 +42,7 @@ static char branch_colors[][COLOR_MAXLEN] = {
GIT_COLOR_NORMAL, /* LOCAL */
GIT_COLOR_GREEN,/* CURRENT */
GIT_COLOR_BLUE, /* UPSTREAM */
+ GIT_COLOR_YELLOW, /* PUBLISH */
};
enum color_branch {
BRANCH_COLOR_RESET = 0,
@@ -49,7 +50,8 @@ enum color_branch {
BRANCH_COLOR_REMOTE = 2,
BRANCH_COLOR_LOCAL = 3,
BRANCH_COLOR_CURRENT = 4,
- BRANCH_COLOR_UPSTREAM = 5
+ BRANCH_COLOR_UPSTREAM = 5,
+ BRANCH_COLOR_PUBLISH = 6
};
static enum merge_filter {
@@ -76,6 +78,8 @@ static int parse_branch_color_slot(const char *var, int ofs)
return BRANCH_COLOR_CURRENT;
if (!strcasecmp(var+ofs, upstream))
return BRANCH_COLOR_UPSTREAM;
+ if (!strcasecmp(var+ofs, publish))
+ return BRANCH_COLOR_PUBLISH;
return -1;
}
@@ -424,17 +428,37 @@ static void fill_tracking_info(struct strbuf *stat, const
char *branch_name,
struct branch *branch = branch_get(branch_name);
struct strbuf fancy = STRBUF_INIT;
+ if (!branch)
+ return;
+
if (!stat_tracking_info(branch, ours, theirs)) {
- if (branch branch-merge branch-merge[0]-dst
- show_upstream_ref) {
+ if (!show_upstream_ref)
+ return;
+ if (branch-merge branch-merge[0]-dst) {
ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
if (want_color(branch_use_color))
- strbuf_addf(stat, [%s%s%s] ,
+ strbuf_addf(fancy, %s%s%s,
branch_get_color(BRANCH_COLOR_UPSTREAM),
ref,
branch_get_color(BRANCH_COLOR_RESET));
else
- strbuf_addf(stat, [%s] , ref);
+ strbuf_addstr(fancy, ref);
+ }
+ if (branch-push.dst) {
+ ref = shorten_unambiguous_ref(branch-push.dst, 0);
+ if (fancy.len)
+ strbuf_addstr(fancy, , );
+ if (want_color(branch_use_color))
+ strbuf_addf(fancy, %s%s%s,
+
branch_get_color(BRANCH_COLOR_PUBLISH),
+ ref,
branch_get_color(BRANCH_COLOR_RESET));
+ else
+ strbuf_addstr(fancy, ref);
}
+ if (!fancy.len)
+ return;
+ strbuf_addf(stat, _([%s]), fancy.buf);
+ strbuf_release(fancy);
+ strbuf_addch(stat, ' ');
return;
}
@@ -446,6 +470,17 @@ static void fill_tracking_info(struct strbuf *stat, const
char *branch_name,
ref,
branch_get_color(BRANCH_COLOR_RESET));
else
strbuf_addstr(fancy, ref);
+ if (branch-push.dst) {
+ ref = shorten_unambiguous_ref(branch-push.dst, 0);
+ if (fancy.len)
+ strbuf_addstr(fancy, , );
+ if (want_color(branch_use_color))
+ strbuf_addf(fancy, %s%s%s,
+
branch_get_color(BRANCH_COLOR_PUBLISH),
+ ref,
branch_get_color(BRANCH_COLOR_RESET));
+ else
+ strbuf_addstr(fancy, ref);
+ }
}
if (!ours) {
--
1.8.4-337-g7358a66-dirty
--
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 1/6] push: trivial reorganization
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/push.c | 35 +++
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 04f0eaf..5dc06a3 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -113,20 +113,11 @@ static NORETURN int die_push_simple(struct branch
*branch, struct remote *remote
remote-name, branch-name, advice_maybe);
}
-static const char message_detached_head_die[] =
- N_(You are not currently on a branch.\n
- To push the history leading to the current (detached HEAD)\n
- state now, use\n
- \n
- git push %s HEAD:name-of-remote-branch\n);
-
static void setup_push_upstream(struct remote *remote, struct branch *branch,
int triangular)
{
struct strbuf refspec = STRBUF_INIT;
- if (!branch)
- die(_(message_detached_head_die), remote-name);
if (!branch-merge_nr || !branch-merge || !branch-remote_name)
die(_(The current branch %s has no upstream branch.\n
To push the current branch and set the remote as upstream,
use\n
@@ -156,8 +147,6 @@ static void setup_push_upstream(struct remote *remote,
struct branch *branch,
static void setup_push_current(struct remote *remote, struct branch *branch)
{
- if (!branch)
- die(_(message_detached_head_die), remote-name);
add_refspec(branch-name);
}
@@ -191,9 +180,23 @@ static int is_workflow_triangular(struct remote *remote)
return (fetch_remote fetch_remote != remote);
}
-static void setup_default_push_refspecs(struct remote *remote)
+static const char message_detached_head_die[] =
+ N_(You are not currently on a branch.\n
+ To push the history leading to the current (detached HEAD)\n
+ state now, use\n
+ \n
+ git push %s HEAD:name-of-remote-branch\n);
+
+static struct branch *get_current_branch(struct remote *remote)
{
struct branch *branch = branch_get(NULL);
+ if (!branch)
+ die(_(message_detached_head_die), remote-name);
+ return branch;
+}
+
+static void setup_default_push_refspecs(struct remote *remote)
+{
int triangular = is_workflow_triangular(remote);
switch (push_default) {
@@ -208,17 +211,17 @@ static void setup_default_push_refspecs(struct remote
*remote)
case PUSH_DEFAULT_SIMPLE:
if (triangular)
- setup_push_current(remote, branch);
+ setup_push_current(remote, get_current_branch(remote));
else
- setup_push_upstream(remote, branch, triangular);
+ setup_push_upstream(remote, get_current_branch(remote),
triangular);
break;
case PUSH_DEFAULT_UPSTREAM:
- setup_push_upstream(remote, branch, triangular);
+ setup_push_upstream(remote, get_current_branch(remote),
triangular);
break;
case PUSH_DEFAULT_CURRENT:
- setup_push_current(remote, branch);
+ setup_push_current(remote, get_current_branch(remote));
break;
case PUSH_DEFAULT_NOTHING:
--
1.8.4-337-g7358a66-dirty
--
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 5/6] push: add --set-publish option
To setup publish tracking branch, like 'git branch --set-publish'.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
Documentation/git-push.txt | 9 +-
builtin/push.c | 2 ++
t/t5529-push-publish.sh| 70 ++
transport.c| 28 +--
transport.h| 1 +
5 files changed, 100 insertions(+), 10 deletions(-)
create mode 100755 t/t5529-push-publish.sh
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index f7dfe48..6f3885d 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,8 @@ SYNOPSIS
[verse]
'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run]
[--receive-pack=git-receive-pack]
- [--repo=repository] [-f | --force] [--prune] [-v | --verbose] [-u
| --set-upstream]
+ [--repo=repository] [-f | --force] [--prune] [-v | --verbose]
+ [-u | --set-upstream] [-p | --set-publish]
[--no-verify] [repository [refspec...]]
DESCRIPTION
@@ -171,6 +172,12 @@ useful if you write an alias or script around 'git push'.
linkgit:git-pull[1] and other commands. For more information,
see 'branch.name.merge' in linkgit:git-config[1].
+-p::
+--set-publish::
+ For every branch that is up to date or successfully pushed, add
+ publish branch tracking reference, used by argument-less
+ linkgit:git-pull[1] and other commands.
+
--[no-]thin::
These options are passed to linkgit:git-send-pack[1]. A thin transfer
significantly reduces the amount of sent data when the sender and
diff --git a/builtin/push.c b/builtin/push.c
index f2deddf..1c184b3 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -473,6 +473,8 @@ int cmd_push(int argc, const char **argv, const char
*prefix)
OPT_STRING( 0 , exec, receivepack, receive-pack,
N_(receive pack program)),
OPT_BIT('u', set-upstream, flags, N_(set upstream for git
pull/status),
TRANSPORT_PUSH_SET_UPSTREAM),
+ OPT_BIT('p', set-publish, flags, N_(set publish for git
pull/status),
+ TRANSPORT_PUSH_SET_PUBLISH),
OPT_BOOL(0, progress, progress, N_(force progress
reporting)),
OPT_BIT(0, prune, flags, N_(prune locally removed refs),
TRANSPORT_PUSH_PRUNE),
diff --git a/t/t5529-push-publish.sh b/t/t5529-push-publish.sh
new file mode 100755
index 000..2037026
--- /dev/null
+++ b/t/t5529-push-publish.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+
+test_description='push with --set-publish'
+
+. ./test-lib.sh
+
+test_expect_success 'setup bare parent' '
+ git init --bare parent
+ git remote add publish parent
+'
+
+test_expect_success 'setup local commit' '
+ echo content file
+ git add file
+ git commit -m one
+'
+
+check_config() {
+ (echo $2; echo $3) expect.$1
+ (git config branch.$1.pushremote
+git config branch.$1.push) actual.$1
+ test_cmp expect.$1 actual.$1
+}
+
+test_expect_success 'push -p master:master' '
+ git push -p publish master:master
+ check_config master publish refs/heads/master
+'
+
+test_expect_success 'push -u master:other' '
+ git push -p publish master:other
+ check_config master publish refs/heads/other
+'
+
+test_expect_success 'push -p --dry-run master:otherX' '
+ git push -p --dry-run publish master:otherX
+ check_config master publish refs/heads/other
+'
+
+test_expect_success 'push -p master2:master2' '
+ git branch master2
+ git push -p publish master2:master2
+ check_config master2 publish refs/heads/master2
+'
+
+test_expect_success 'push -p master2:other2' '
+ git push -p publish master2:other2
+ check_config master2 publish refs/heads/other2
+'
+
+test_expect_success 'push -p :master2' '
+ git push -p publish :master2
+ check_config master2 publish refs/heads/other2
+'
+
+test_expect_success 'push -u --all' '
+ git branch all1
+ git branch all2
+ git push -p --all
+ check_config all1 publish refs/heads/all1
+ check_config all2 publish refs/heads/all2
+'
+
+test_expect_success 'push -p HEAD' '
+ git checkout -b headbranch
+ git push -p publish HEAD
+ check_config headbranch publish refs/heads/headbranch
+'
+
+test_done
diff --git a/transport.c b/transport.c
index e15db98..d796732 100644
--- a/transport.c
+++ b/transport.c
@@ -140,8 +140,8 @@ static void insert_packed_refs(const char *packed_refs,
struct ref **list)
}
}
-static void set_upstreams(struct transport *transport, struct ref *refs,
- int pretend)
+static void set_tracking(struct transport *transport, struct ref *refs,
+ int pretend, int publish)
{
struct ref *ref;
for (ref = refs; ref; ref = ref-next) {
@@ -176,12 +176,18 @@ static void
[PATCH 0/6] Introduce publish tracking branch
Hi, As it has been discussed before, our support for triangular workflows is lacking, and the following patch series aims to improve that situation. We have the concept of upstream branch (e.g. 'origin/master') which is to where our topic branches eventually should be merged to, so it makes sense that 'git rebase' uses that as the destination, but most people would not push to such upstream branch, they would push to a publish branch (e.g. 'github/feature-a'). We could set our upstream to the place we push, and 'git push' would be able to use that as default, and 'git branch --vv' would show how ahead/behind we are in comparisson to that branch, but then 'git rebase' (or 'git merge') would be using the wrong branch. This patch series adds: 1) git push --set-publish 2) git branch --set-publish 3) git branch -vv # uses and shows the publish branch when configured After this, it becomes much easier to track branches in a triangular workflow. master e230c56 [origin/master, gh/master] Git 1.8.4 * fc/publish 0a105fd [master, gh/fc/publish: ahead 1] branch: display publish branch fc/branch/fast 177dcad [master, gh/fc/branch/fast] branch: reorganize verbose options fc/trivial f289b9a [master: ahead 7] branch: trivial style fix fc/leaksd101af4 [master: ahead 2] read-cache: plug a possible leak stable e230c56 Git 1.8.4 Felipe Contreras (6): push: trivial reorganization Add concept of 'publish' branch branch: allow configuring the publish branch t: branch add publish branch tests push: add --set-publish option branch: display publish branch Documentation/git-branch.txt | 11 + Documentation/git-push.txt | 9 +++- branch.c | 43 ++ branch.h | 2 + builtin/branch.c | 101 +++ builtin/push.c | 52 ++ remote.c | 34 --- remote.h | 4 ++ t/t3200-branch.sh| 76 t/t5529-push-publish.sh | 70 ++ transport.c | 28 transport.h | 1 + 12 files changed, 388 insertions(+), 43 deletions(-) create mode 100755 t/t5529-push-publish.sh -- 1.8.4-337-g7358a66-dirty -- 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 2/6] Add concept of 'publish' branch
The upstream branch is:
branch.$name.remote
branch.$name.merge
The publish branch is:
branch.$name.pushremote
branch.$name.push
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/push.c | 19 +++
remote.c | 34 --
remote.h | 4
3 files changed, 47 insertions(+), 10 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 5dc06a3..f2deddf 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -150,6 +150,20 @@ static void setup_push_current(struct remote *remote,
struct branch *branch)
add_refspec(branch-name);
}
+static void setup_push_simple(struct remote *remote, struct branch *branch,
+ int triangular)
+{
+ if (branch-push_name) {
+ struct strbuf refspec = STRBUF_INIT;
+ strbuf_addf(refspec, %s:%s, branch-name, branch-push_name);
+ add_refspec(refspec.buf);
+ } else if (triangular) {
+ setup_push_current(remote, branch);
+ } else {
+ setup_push_upstream(remote, branch, triangular);
+ }
+}
+
static char warn_unspecified_push_default_msg[] =
N_(push.default is unset; its implicit value is changing in\n
Git 2.0 from 'matching' to 'simple'. To squelch this message\n
@@ -210,10 +224,7 @@ static void setup_default_push_refspecs(struct remote
*remote)
break;
case PUSH_DEFAULT_SIMPLE:
- if (triangular)
- setup_push_current(remote, get_current_branch(remote));
- else
- setup_push_upstream(remote, get_current_branch(remote),
triangular);
+ setup_push_simple(remote, get_current_branch(remote),
triangular);
break;
case PUSH_DEFAULT_UPSTREAM:
diff --git a/remote.c b/remote.c
index efcba93..04c7ed9 100644
--- a/remote.c
+++ b/remote.c
@@ -350,13 +350,17 @@ static int handle_config(const char *key, const char
*value, void *cb)
explicit_default_remote_name = 1;
}
} else if (!strcmp(subkey, .pushremote)) {
+ if (git_config_string(branch-pushremote_name, key,
value))
+ return -1;
if (branch == current_branch)
- if (git_config_string(pushremote_name, key,
value))
- return -1;
+ pushremote_name =
xstrdup(branch-pushremote_name);
} else if (!strcmp(subkey, .merge)) {
if (!value)
return config_error_nonbool(key);
add_merge(branch, xstrdup(value));
+ } else if (!strcmp(subkey, .push)) {
+ if (git_config_string(branch-push_name, key, value))
+ return -1;
}
return 0;
}
@@ -1492,6 +1496,14 @@ struct branch *branch_get(const char *name)
}
}
}
+ if (ret ret-pushremote_name) {
+ struct remote *pushremote;
+ pushremote = pushremote_get(ret-pushremote_name);
+ ret-push.src = xstrdup(ret-push_name);
+ if (remote_find_tracking(pushremote, ret-push)
+!strcmp(ret-pushremote_name, .))
+ ret-push.dst = xstrdup(ret-push_name);
+ }
return ret;
}
@@ -1694,6 +1706,15 @@ int ref_newer(const unsigned char *new_sha1, const
unsigned char *old_sha1)
return found;
}
+static char *get_base(struct branch *branch)
+{
+ if (branch-push.dst)
+ return branch-push.dst;
+ if (branch-merge branch-merge[0] branch-merge[0]-dst)
+ return branch-merge[0]-dst;
+ return NULL;
+}
+
/*
* Return true if there is anything to report, otherwise false.
*/
@@ -1710,15 +1731,16 @@ int stat_tracking_info(struct branch *branch, int
*num_ours, int *num_theirs)
* Nothing to report unless we are marked to build on top of
* somebody else.
*/
- if (!branch ||
- !branch-merge || !branch-merge[0] || !branch-merge[0]-dst)
+ if (!branch)
+ return 0;
+ base = get_base(branch);
+ if (!base)
return 0;
/*
* If what we used to build on no longer exists, there is
* nothing to report.
*/
- base = branch-merge[0]-dst;
if (read_ref(base, sha1))
return 0;
theirs = lookup_commit_reference(sha1);
@@ -1781,7 +1803,7 @@ int format_tracking_info(struct branch *branch, struct
strbuf *sb)
if (!stat_tracking_info(branch, num_ours, num_theirs))
return 0;
- base = branch-merge[0]-dst;
+ base = get_base(branch);
base =
Re: [PATCH 2/6] Add concept of 'publish' branch
On Sun, Sep 1, 2013 at 4:26 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
The upstream branch is:
branch.$name.remote
branch.$name.merge
The publish branch is:
branch.$name.pushremote
branch.$name.push
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/push.c | 19 +++
remote.c | 34 --
remote.h | 4
3 files changed, 47 insertions(+), 10 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 5dc06a3..f2deddf 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -150,6 +150,20 @@ static void setup_push_current(struct remote *remote,
struct branch *branch)
add_refspec(branch-name);
}
+static void setup_push_simple(struct remote *remote, struct branch *branch,
+ int triangular)
+{
+ if (branch-push_name) {
+ struct strbuf refspec = STRBUF_INIT;
+ strbuf_addf(refspec, %s:%s, branch-name,
branch-push_name);
+ add_refspec(refspec.buf);
strbuf_release(refspec);
+ } else if (triangular) {
+ setup_push_current(remote, branch);
+ } else {
+ setup_push_upstream(remote, branch, triangular);
+ }
+}
+
static char warn_unspecified_push_default_msg[] =
N_(push.default is unset; its implicit value is changing in\n
Git 2.0 from 'matching' to 'simple'. To squelch this message\n
@@ -210,10 +224,7 @@ static void setup_default_push_refspecs(struct remote
*remote)
break;
case PUSH_DEFAULT_SIMPLE:
- if (triangular)
- setup_push_current(remote,
get_current_branch(remote));
- else
- setup_push_upstream(remote,
get_current_branch(remote), triangular);
+ setup_push_simple(remote, get_current_branch(remote),
triangular);
break;
case PUSH_DEFAULT_UPSTREAM:
diff --git a/remote.c b/remote.c
index efcba93..04c7ed9 100644
--- a/remote.c
+++ b/remote.c
@@ -350,13 +350,17 @@ static int handle_config(const char *key, const char
*value, void *cb)
explicit_default_remote_name = 1;
}
} else if (!strcmp(subkey, .pushremote)) {
+ if (git_config_string(branch-pushremote_name, key,
value))
+ return -1;
if (branch == current_branch)
- if (git_config_string(pushremote_name, key,
value))
- return -1;
+ pushremote_name =
xstrdup(branch-pushremote_name);
} else if (!strcmp(subkey, .merge)) {
if (!value)
return config_error_nonbool(key);
add_merge(branch, xstrdup(value));
+ } else if (!strcmp(subkey, .push)) {
+ if (git_config_string(branch-push_name, key, value))
+ return -1;
}
return 0;
}
@@ -1492,6 +1496,14 @@ struct branch *branch_get(const char *name)
}
}
}
+ if (ret ret-pushremote_name) {
+ struct remote *pushremote;
+ pushremote = pushremote_get(ret-pushremote_name);
+ ret-push.src = xstrdup(ret-push_name);
+ if (remote_find_tracking(pushremote, ret-push)
+!strcmp(ret-pushremote_name, .))
+ ret-push.dst = xstrdup(ret-push_name);
+ }
return ret;
}
@@ -1694,6 +1706,15 @@ int ref_newer(const unsigned char *new_sha1, const
unsigned char *old_sha1)
return found;
}
+static char *get_base(struct branch *branch)
+{
+ if (branch-push.dst)
+ return branch-push.dst;
+ if (branch-merge branch-merge[0] branch-merge[0]-dst)
+ return branch-merge[0]-dst;
+ return NULL;
+}
+
/*
* Return true if there is anything to report, otherwise false.
*/
@@ -1710,15 +1731,16 @@ int stat_tracking_info(struct branch *branch, int
*num_ours, int *num_theirs)
* Nothing to report unless we are marked to build on top of
* somebody else.
*/
- if (!branch ||
- !branch-merge || !branch-merge[0] || !branch-merge[0]-dst)
+ if (!branch)
+ return 0;
+ base = get_base(branch);
+ if (!base)
return 0;
/*
* If what we used to build on no longer exists, there is
* nothing to report.
*/
- base = branch-merge[0]-dst;
if (read_ref(base, sha1))
return 0;
theirs = lookup_commit_reference(sha1);
@@ -1781,7 +1803,7 @@ int
Re: [PATCH v3 08/11] t6050-replace: check that -f option bypasses the type check
From: Eric Sunshine sunsh...@sunshineco.com
On Sat, Aug 31, 2013 at 3:12 PM, Christian Couder
chrisc...@tuxfamily.org wrote:
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
t/t6050-replace.sh | 6 ++
1 file changed, 6 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 05be228..0b07a0b 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,6 +276,12 @@ test_expect_success 'replaced and replacement objects
must be of the same type'
grep $BLOB. points to a replacement object of type .blob err
'
+test_expect_success '-f option bypasses the type check' '
+ git replace -f mytag $HASH1 2err
+ git replace -f HEAD^{tree} HEAD~1 2err
+ git replace -f HEAD^ $BLOB 2err
+'
Is there a non-obvious reason you are redirecting stderr to a file in
this test? Unlike the test added earlier, this one never consults the
error output. By dropping this apparently unnecessary redirection,
diagnosis of a regression potentially becomes simpler since any error
output from git-replace will become visible when the test is run
verbosely.
You are right! I will drop these redirections.
Thanks,
Christian.
--
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 v3 11/11] t6050-replace: use some long option names
From: Philip Oakley philipoak...@iee.org So that they are tested a litlle bit too. s /litlle/little/ Thanks, Christian. -- 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 v3 05/11] Documentation/replace: add Creating Replacement Objects section
From: Philip Oakley philipoak...@iee.org From: Christian Couder chrisc...@tuxfamily.org +CREATING REPLACEMENT OBJECTS + + +linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and +linkgit:git-rebase[1], Let's not forget the obvious 'git commit' or 'git merge' on a temporary branch for creating a replacement commit. As it is obvious, and as it is somehow addressed in the below part of this section, I don't think it is worth talking about git commit or git merge or git cherry-pick or any other command. In particular we need to have covered the alternate to a graft of A B C (i.e. A is now a merge of B C) if we are to deprecate grafts with any conviction. (https://git.wiki.kernel.org/index.php/GraftPoint) Adding such an example in a new EXAMPLE section would address this better. If people agree I will do it in a following patch. among other git commands, can be used to create +replacement objects from existing objects. + +If you want to replace many blobs, trees or commits that are part of a +string of commits, you may just want to create a replacement string of +commits and then only replace the commit at the tip of the target +string of commits with the commit at the tip of the replacement string +of commits. Thanks, Christian. -- 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 2/6] Add concept of 'publish' branch
On Sun, Sep 1, 2013 at 3:41 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Sun, Sep 1, 2013 at 4:26 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
+static void setup_push_simple(struct remote *remote, struct branch *branch,
+ int triangular)
+{
+ if (branch-push_name) {
+ struct strbuf refspec = STRBUF_INIT;
+ strbuf_addf(refspec, %s:%s, branch-name,
branch-push_name);
+ add_refspec(refspec.buf);
strbuf_release(refspec);
Actually no, wee need that buffer. See setup_push_upstream().
--
Felipe Contreras
--
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 v3 07/11] Documentation/replace: tell that -f option bypasses the type check
From: Philip Oakley philipoak...@iee.org From: Christian Couder chrisc...@tuxfamily.org The replaced object and the replacement object must be of the same type. -There is no other restriction on them. +This restriction can be bypassed using `-f`. Unless `-f` is given, the 'replace' reference must not yet exist. +There is no other restriction on the replaced and replacement objects. Is this trying to allude to the fact that merge commits may be exchanged with non-merge commits? I strongly believe that this ability to exchange merge and non-merge commits should be stated _explicitly_ to counteract the false beliefs that are listed out on the internet. Maybe we can show that in an example. But I think the patch is quite clear as it is and should be enough. If we really want to correct some false beliefs, the best would be to state the truth where those false beliefs are stated. It's probably better stated in a separate patch for that explicit purpose to avoid mixed messages within this commit. If people agree, I will add a another patch with an example in an EXAMPLE section. Thanks, Christian. -- 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 v3 01/11] replace: forbid replacing an object with one of a different type
From: Philip Oakley philipoak...@iee.org Sorry for not replying earlier in the series. From: Christian Couder chrisc...@tuxfamily.org Users replacing an object with one of a different type were not prevented to do so, even if it was obvious, and stated in the doc, that bad things would result from doing that. To avoid mistakes, it is better to just forbid that though. If one object is replaced with one of a different type, the only way to keep the history valid is to also replace all the other objects that point to the replaced object. Isn't this a recursion problem? Taken in that order one unravels the whole DAG. However if considered in the reverse direction, one can replace an existing object within the DAG with a carefully crafted alternative of the same type, but which then wrongly references other dangling objects which are then replaced by objects which have the right type (this last replacement requires -f force). I am not sure I understand what you are saying. Anyway in a previous version of this patch I tried to be more explicit about this, but Junio basically said that he found no value in discussing this more explicitely... Thanks, Christian. -- 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
[RFC] Running a hook on reset
Hi, I propose adding a hook that is run after git reset. My personal use case for that is that I am indexing my checkouts using global (a tool similar to ctags, cscope, ...) and I really like that index to be up2date all the time. I can catch most cases where the work tree changes from git via the post-checkout, post-merge, post-rewrite hooks. The only frequent case I don't know how to trigger a reindex is git reset --hard. Another related usecases I can imagine is people wanting to check that there is nothing important/irrecoverable in the current checkout before resetting it, but that would obviously be a pre-reset hook. Any opinions about the feature before I start writing a patch? Greetings, Andres Freund -- 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 v3 3/4] get rid of git submodule summary --for-status
Am 31.08.2013 19:08, schrieb brian m. carlson: On Fri, Aug 30, 2013 at 10:08:53PM +0200, Jens Lehmann wrote: Am 30.08.2013 21:51, schrieb Jens Lehmann: Am 30.08.2013 21:40, schrieb Jens Lehmann: Am 29.08.2013 23:23, schrieb Matthieu Moy: Jens Lehmann jens.lehm...@web.de writes: Am 29.08.2013 15:05, schrieb Matthieu Moy: Because of the missing quotes around $for_status, it seems the test is unconditionnaly true: $ test -n t ; echo $? 0 $ test -n ; echo $? 0 Right you are, I did not notice the missing in my review. Looks like we also should add one or more tests making sure that submodule summary and status never honor the ignore settings. How do we want to handle this? I can send a reroll and include some new tests, but if this code is going away, then there's no point. A reroll would be great, as I think your patch is a bugfix that should go in rather soonish no matter how we continue with the comment signs. Two new tests (one for submodule summary and one for submodule status) with both the global ignore setting and a submodule specific one set to all showing no impact on the output would suffice (and trigger the then also fixed missing bug ;-). -- 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: THANK YOU
i have a business proposal for you, write me back for more info. -Sent from my ipad. -- 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 v3 01/11] replace: forbid replacing an object with one of a different type
From: Christian Couder chrisc...@tuxfamily.org From: Philip Oakley philipoak...@iee.org Sorry for not replying earlier in the series. From: Christian Couder chrisc...@tuxfamily.org Users replacing an object with one of a different type were not prevented to do so, even if it was obvious, and stated in the doc, that bad things would result from doing that. To avoid mistakes, it is better to just forbid that though. If one object is replaced with one of a different type, the only way to keep the history valid is to also replace all the other objects that point to the replaced object. Isn't this a recursion problem? Taken in that order one unravels the whole DAG. However if considered in the reverse direction, one can replace an existing object within the DAG with a carefully crafted alternative of the same type, but which then wrongly references other dangling objects which are then replaced by objects which have the right type (this last replacement requires -f force). I am not sure I understand what you are saying. Anyway in a previous version of this patch I tried to be more explicit about this, but Junio basically said that he found no value in discussing this more explicitely... I would agree that it's not worth discussing it more explicitly. My comment was more about the direction of the line of reasoning which I felt was a bit Catch 22 when starting from an existing complete DAG (no garbage) and attempting to replace an object with another of a different type and still have a valid DAG. The construction of the replacement items needs to be in the right order if one of the replacements is of the 'wrong' type (such a construction requires the creation or uses, and ultimately replacement of, extraneous objects that aren't (yet) in the DAG). But as already been said that's a problem for the user of the --force option ;-) Philip Thanks, Christian. -- 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 v3 0/2] submodule: Don't print status output with submodule.name.ignore=all
There are configuration options for each submodule that specify under what circumstances git status should display output for that submodule. Unfortunately, these settings were not being respected, and as such the tests were marked TODO. This patch series consists of two patches: the first is a fix for a confusing variable name, and the latter actually makes git status not output the submodule information. The tests have been updated accordingly, and additional tests have been included to ensure that git submodule summary is not affected by these changes unless the --for-status option is given. Changes from v2: * Add tests to ensure that git submodule summary is not affected. * Fix bug that caused git submodule summary to be affected. Changes from v1: * Handle moved submodules by not ignoring them. * Use sm_path instead of path. * Only ignore when --for-status is given. brian m. carlson (2): submodule: fix confusing variable name submodule: don't print status output with ignore=all git-submodule.sh | 15 +++ t/t7401-submodule-summary.sh | 30 ++ t/t7508-status.sh| 4 ++-- 3 files changed, 43 insertions(+), 6 deletions(-) -- 1.8.4.rc3 -- 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 v3 1/2] submodule: fix confusing variable name
cmd_summary reads the output of git diff, but reads in the submodule path into a
variable called name. Since this variable does not contain the name of the
submodule, but the path, rename it to be clearer what data it actually holds.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
git-submodule.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197..38520db 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1032,13 +1032,13 @@ cmd_summary() {
# Get modified modules cared by user
modules=$(git $diff_cmd $cached --ignore-submodules=dirty --raw $head
-- $@ |
sane_egrep '^:([0-7]* )?16' |
- while read mod_src mod_dst sha1_src sha1_dst status name
+ while read mod_src mod_dst sha1_src sha1_dst status sm_path
do
# Always show modules deleted or type-changed
(blob-module)
- test $status = D -o $status = T echo $name
continue
+ test $status = D -o $status = T echo $sm_path
continue
# Also show added or modified modules which are checked
out
- GIT_DIR=$name/.git git-rev-parse --git-dir /dev/null
21
- echo $name
+ GIT_DIR=$sm_path/.git git-rev-parse --git-dir
/dev/null 21
+ echo $sm_path
done
)
--
1.8.4.rc3
--
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 v3 2/2] submodule: don't print status output with ignore=all
git status prints information for submodules, but it should ignore the status of
those which have submodule.name.ignore set to all. Fix it so that it does
properly ignore those which have that setting either in .git/config or in
.gitmodules.
Not ignored are submodules that are added, deleted, or moved (which is
essentially a combination of the first two) because it is not easily possible to
determine the old path once a move has occurred, nor is it easily possible to
detect which adds and deletions are moves and which are not. This also
preserves the previous behavior of always listing modules which are to be
deleted.
Tests are included which verify that this change has no effect on git submodule
summary without the --for-status option.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
git-submodule.sh | 7 +++
t/t7401-submodule-summary.sh | 30 ++
t/t7508-status.sh| 4 ++--
3 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 38520db..004b21c 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1036,6 +1036,13 @@ cmd_summary() {
do
# Always show modules deleted or type-changed
(blob-module)
test $status = D -o $status = T echo $sm_path
continue
+ # Respect the ignore setting for --for-status.
+ if test -n $for_status
+ then
+ name=$(module_name $sm_path)
+ ignore_config=$(get_submodule_config $name
ignore none)
+ test $status != A -a $ignore_config = all
continue
+ fi
# Also show added or modified modules which are checked
out
GIT_DIR=$sm_path/.git git-rev-parse --git-dir
/dev/null 21
echo $sm_path
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index ac2434c..ca9441e 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -104,6 +104,36 @@ EOF
test_cmp expected actual
+test_expect_success '.gitmodules ignore=all has no effect'
+ git config --add -f .gitmodules submodule.sm1.ignore all
+ git config --add -f .gitmodules submodule.sm1.path sm1
+ git submodule summary actual
+ cat expected -EOF
+* sm1 $head1...$head2 (1):
+ Add foo3
+
+EOF
+ test_cmp expected actual
+ git config -f .gitmodules --remove-section submodule.sm1
+
+
+test_expect_success '.git/config ignore=all has no effect'
+ git config --add -f .gitmodules submodule.sm1.ignore none
+ git config --add -f .gitmodules submodule.sm1.path sm1
+ git config --add submodule.sm1.ignore all
+ git config --add submodule.sm1.path sm1
+ git submodule summary actual
+ cat expected -EOF
+* sm1 $head1...$head2 (1):
+ Add foo3
+
+EOF
+ test_cmp expected actual
+ git config --remove-section submodule.sm1
+ git config -f .gitmodules --remove-section submodule.sm1
+
+
+
commit_file sm1
head3=$(
cd sm1
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index ac3d0fe..fb89fb9 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1316,7 +1316,7 @@ test_expect_success --ignore-submodules=all suppresses
submodule summary '
test_i18ncmp expect output
'
-test_expect_failure '.gitmodules ignore=all suppresses submodule summary' '
+test_expect_success '.gitmodules ignore=all suppresses submodule summary' '
git config --add -f .gitmodules submodule.subname.ignore all
git config --add -f .gitmodules submodule.subname.path sm
git status output
@@ -1324,7 +1324,7 @@ test_expect_failure '.gitmodules ignore=all suppresses
submodule summary' '
git config -f .gitmodules --remove-section submodule.subname
'
-test_expect_failure '.git/config ignore=all suppresses submodule summary' '
+test_expect_success '.git/config ignore=all suppresses submodule summary' '
git config --add -f .gitmodules submodule.subname.ignore none
git config --add -f .gitmodules submodule.subname.path sm
git config --add submodule.subname.ignore all
--
1.8.4.rc3
--
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 v3 07/11] Documentation/replace: tell that -f option bypasses the type check
From: Christian Couder chrisc...@tuxfamily.org From: Philip Oakley philipoak...@iee.org From: Christian Couder chrisc...@tuxfamily.org The replaced object and the replacement object must be of the same type. -There is no other restriction on them. +This restriction can be bypassed using `-f`. Unless `-f` is given, the 'replace' reference must not yet exist. +There is no other restriction on the replaced and replacement objects. Is this trying to allude to the fact that merge commits may be exchanged with non-merge commits? I strongly believe that this ability to exchange merge and non-merge commits should be stated _explicitly_ to counteract the false beliefs that are listed out on the internet. Maybe we can show that in an example. But I think the patch is quite clear as it is and should be enough. If we really want to correct some false beliefs, the best would be to state the truth where those false beliefs are stated. I've added a sub-comment to the original SO post that started this thread (my post $gmane/231598 - SO/a/18027030/717355), but given the guy's blog has comments going back to 2009 I suspect its a bit of a http://xkcd.com/386/ task, hence my desire that it's explicitly mentioned in the 'replace' documentation. In addition, if the guy doesn't correct his post I'll mark it down in a couple of days to make it plain to other readers it's in error. The creation of a (merge?) commit that's equivalent to a graft line isn't something that jumps out (to me) as an easy couple lines of bash script. A 'graft2replace' script (or 'git-graft' command) which takes an existing graft file (or command line list) and creates the replacement objects and then does the replace, all while still in a dirty tree would be the holy grail for properly deprecating grafts (which are sooo easy to create) It's probably better stated in a separate patch for that explicit purpose to avoid mixed messages within this commit. If people agree, I will add a another patch with an example in an EXAMPLE section. Thanks, Christian. Thanks for your work on this. Philip -- 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] {fetch,receive}-pack: drop unpack-objects, delay loosing objects until the end
Current code peaks into the transfered pack's header, if the number of
objects is under a limit, unpack-objects is called to handle the rest,
otherwise index-pack is. This patch makes fetch-pack use index-pack
unconditionally, then turn objects loose and remove the pack at the
end. unpack-objects is deprecated and may be removed in future.
There are a few reasons for this:
- down to two code paths to maintain regarding pack reading
(sha1_file.c and index-pack.c). When .pack v4 comes, we don't need
to duplicate work in index-pack and unpack-objects.
- the number of objects should not be the only indicator for
unpacking. If there are a few large objects in the pack, the pack
should be kept anyway. Keeping the pack lets us examine the whole
pack later and make a better decision.
- by going through index-pack first, then unpack, we pay extra cost
for completing a thin pack into a full one. But compared to fetch's
total time, it should not be noticeable because unpack-objects is
only called when the pack contains a small number of objects.
- unpack-objects does not support streaming large blobs. Granted
force_object_loose(), which is used by this patch, does not either.
But it'll be easier to do so because sha1_file.c interface supports
streaming.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/receive-pack.c | 120 +
cache.h| 1 +
fetch-pack.c | 77 +++
sha1_file.c| 70 -
4 files changed, 128 insertions(+), 140 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e3eb5fc..6eb4ffb 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -792,105 +792,49 @@ static struct command *read_head_info(void)
return commands;
}
-static const char *parse_pack_header(struct pack_header *hdr)
-{
- switch (read_pack_header(0, hdr)) {
- case PH_ERROR_EOF:
- return eof before pack header was fully read;
-
- case PH_ERROR_PACK_SIGNATURE:
- return protocol error (pack signature mismatch detected);
-
- case PH_ERROR_PROTOCOL:
- return protocol error (pack version unsupported);
-
- default:
- return unknown error in parse_pack_header;
-
- case 0:
- return NULL;
- }
-}
-
static const char *pack_lockfile;
static const char *unpack(int err_fd)
{
- struct pack_header hdr;
- const char *hdr_err;
- char hdr_arg[38];
int fsck_objects = (receive_fsck_objects = 0
? receive_fsck_objects
: transfer_fsck_objects = 0
? transfer_fsck_objects
: 0);
- hdr_err = parse_pack_header(hdr);
- if (hdr_err) {
- if (err_fd 0)
- close(err_fd);
- return hdr_err;
- }
- snprintf(hdr_arg, sizeof(hdr_arg),
- --pack_header=%PRIu32,%PRIu32,
- ntohl(hdr.hdr_version), ntohl(hdr.hdr_entries));
-
- if (ntohl(hdr.hdr_entries) unpack_limit) {
- int code, i = 0;
- struct child_process child;
- const char *unpacker[5];
- unpacker[i++] = unpack-objects;
- if (quiet)
- unpacker[i++] = -q;
- if (fsck_objects)
- unpacker[i++] = --strict;
- unpacker[i++] = hdr_arg;
- unpacker[i++] = NULL;
- memset(child, 0, sizeof(child));
- child.argv = unpacker;
- child.no_stdout = 1;
- child.err = err_fd;
- child.git_cmd = 1;
- code = run_command(child);
- if (!code)
- return NULL;
- return unpack-objects abnormal exit;
- } else {
- const char *keeper[7];
- int s, status, i = 0;
- char keep_arg[256];
- struct child_process ip;
-
- s = sprintf(keep_arg, --keep=receive-pack %PRIuMAX on ,
(uintmax_t) getpid());
- if (gethostname(keep_arg + s, sizeof(keep_arg) - s))
- strcpy(keep_arg + s, localhost);
-
- keeper[i++] = index-pack;
- keeper[i++] = --stdin;
- if (fsck_objects)
- keeper[i++] = --strict;
- keeper[i++] = --fix-thin;
- keeper[i++] = hdr_arg;
- keeper[i++] = keep_arg;
- keeper[i++] = NULL;
- memset(ip, 0, sizeof(ip));
- ip.argv = keeper;
- ip.out = -1;
- ip.err = err_fd;
- ip.git_cmd = 1;
- status = start_command(ip);
- if (status) {
-
Re: [PATCH] {fetch,receive}-pack: drop unpack-objects, delay loosing objects until the end
On Sun, Sep 1, 2013 at 11:05 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: Current code peaks into the transfered pack's header, if the number of s/peaks/peeks/ objects is under a limit, unpack-objects is called to handle the rest, otherwise index-pack is. This patch makes fetch-pack use index-pack unconditionally, then turn objects loose and remove the pack at the end. unpack-objects is deprecated and may be removed in future. There are a few reasons for this: - down to two code paths to maintain regarding pack reading (sha1_file.c and index-pack.c). When .pack v4 comes, we don't need to duplicate work in index-pack and unpack-objects. - the number of objects should not be the only indicator for unpacking. If there are a few large objects in the pack, the pack should be kept anyway. Keeping the pack lets us examine the whole pack later and make a better decision. - by going through index-pack first, then unpack, we pay extra cost for completing a thin pack into a full one. But compared to fetch's total time, it should not be noticeable because unpack-objects is only called when the pack contains a small number of objects. - unpack-objects does not support streaming large blobs. Granted force_object_loose(), which is used by this patch, does not either. But it'll be easier to do so because sha1_file.c interface supports streaming. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com -- 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 1/7] glossary: mention 'treeish' as an alternative to 'tree-ish'
The documentation contains a mix of the two spellings, so include both in the glossary so that a search for either will lead to the definition. Signed-off-by: Richard Hansen rhan...@bbn.com --- Documentation/glossary-content.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index dba5062..0273095 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -486,7 +486,7 @@ should not be combined with other pathspec. with refs to the associated blob and/or tree objects. A def_tree,tree is equivalent to a def_directory,directory. -[[def_tree-ish]]tree-ish:: +[[def_tree-ish]]tree-ish (also treeish):: A def_ref,ref pointing to either a def_commit_object,commit object, a def_tree_object,tree object, or a def_tag_object,tag object pointing to a tag or commit or tree object. -- 1.8.4 -- 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 7/7] glossary: fix and clarify the definition of 'ref'
Signed-off-by: Richard Hansen rhan...@bbn.com --- Documentation/glossary-content.txt | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index a2edcc3..44d524b 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -395,10 +395,20 @@ should not be combined with other pathspec. to the result. [[def_ref]]ref:: - A 40-byte hex representation of a def_SHA1,SHA-1 or a name that - denotes a particular def_object,object. They may be stored in - a file under `$GIT_DIR/refs/` directory, or - in the `$GIT_DIR/packed-refs` file. + A name that begins with `refs/` (e.g. `refs/heads/master`) + that points to an def_object_name,object name or another + ref (the latter is called a def_symref,symbolic ref). + For convenience, a ref can sometimes be abbreviated when used + as an argument to a Git command; see linkgit:gitrevisions[7] + for details. + Refs are stored in the def_repository,repository. ++ +The ref namespace is hierarchical. +Different subhierarchies are used for different purposes (e.g. the +`refs/heads/` hierarchy is used to represent local branches). ++ +There are a few special-purpose refs that do not begin with `refs/`. +The most notable example is `HEAD`. [[def_reflog]]reflog:: A reflog shows the local history of a ref. In other words, -- 1.8.4 -- 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
