Re: [PATCH v3 00/10] transport-helper: updates
On 10/12/2013 03:05 AM, Felipe Contreras wrote: Hi, Here are the patches that allow transport helpers to be completely transparent; renaming branches, deleting them, custom refspecs, --force, --dry-run, reporting forced update, everything works. These patches don't cleanly apply to master anymore; would you be willing to rebase and post a new version? I wanted to test these changes via git-remote-bzr on a bzr repository I'm working on, but unfortunately git-remote-bzr doesn't yet support force pushes. I may look into adding force push support to git-remote-bzr, unless you beat me to it. :) In general these patches look good to me and I'd like to see them merged to master. Thanks, Richard -- 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] commit: Add -f, --fixes commit option to add Fixes: line
One of the uses of the Fixes commit line is so that when we fix a security bug that has been in mainline for a while, it can be tricky to determine whether it should be backported in to the various stable branches. For example, let's suppose the security bug (or any bug, but one of the contexts where this came up was for security fixes) was introduced in 3.5, and backported into the 3.2.x kernel series, but couldn't be applied into the 3.2.0 kernel series. The security fix was introduced in 3.12, and so it would be obvious that it should be backported to the 3.10 kernel series, but it might not be so obvious that it would also be required for the 3.2.x long-term stable series. So the inclusion of the Fixes: line provides this critical bit of information. It's also useful not just for the long-term stable tree maintainers, but the maintainers of distro kernels would also find it to be very useful. I see that there a consistency check that the --fixes argument is a valid commit. But is there/should there be a check that it is an ancestor of the commit being created? Is there/should there be a check that both of these facts remain true if the the commit containing it is rebased, cherry-picked, etc? In workflows that make more use of cherry-picking, it could be that the original buggy commit was cherry-picked to a different branch. In this case the user would probably want to cherry-pick the fixing commit to the other branch, too. But then the commit that it would be fixing would have a different SHA-1 than it did on the original branch. A check that the Fixes: line refers to an ancestor of the current commit could warn against such errors. (In some cases it might be possible to use cherry-pick's -x lines to figure out how to rewrite the Fixes: line, but I doubt that would work often enough to be worthwhile.) I believe that in the discussions we had, it was assumed that the Fixes: line would reference the commit in the mainline kernel tree. i.e., it would always reference the commit which introduced the bug in 3.5, even if the commit-id after the buggy commit was backported to 3.2.x would obviously be different. Presumably the distro kernel maintainer would be able to find the commit in Linus's tree and then try to find the corresponding commit in the distro kernel git tree, probably by doing string searches over git log. We could actually do a much more elegant job if we did have the concept of commit identity (i.e., ChangeID's) baked into git. That way, there would be a constant ChangeID that would remain constant not only across revisions of a patch under development, but also when the commit is cherry picked into stable branches. If we had that, then instead of doing string searches on git log output, we could imagine a web and/or command line interface where given a ChangeID, it would tell you which branches or which tags contained the same semantic patch. Of course, as soon as you do that, then if the multiple commits get squashed together, you might need to have to support multiple ChangeID's associated with one commit, at which point it becomes incompatible with Gerrit's use of this feature. So we could add all sorts of complexity, but it's not obvious to me that it's worth it. First of all, let me show my ignorance. How formalized is the use of metadata lines at the end of a commit message? I don't remember seeing documentation about such lines in general (as opposed to documentation about particular types of lines). Is the format defined well enough that tools that don't know about a particular line could nonetheless preserve it correctly? Is there/should there be a standard recommended order of metadata lines? (For example, should Fixes: lines always appear before Signed-off-by lines, or vice versa?) If so, is it documented somewhere and preserved by tools when such lines are added/modified? Should there be support for querying such lines? Internally inside Google, we have tools that will assist in forward porting local changes from a 3.x based kernel to a 3.y kernel, to make sure that all local changes are properly accounted for and none are accidentally dropped during the rebase operation. So we have various new metadata lines that we add internally, for example: Upstream-3.x-SHA1: commit-id for commits in newer kernels that have been backported Origin-3.x-SHA1: commit-id to indicate the commit-id of a patch that was forward ported as part of a rebase operation from 3.x to 3.9 Upstream-Dropped-3.x-SHA1: commit-id As part of an empty commit to indicate that a patch that was originally in our tree, has since been pushed upstream, so we can drop it as part of the rebase to the 3.y kernel. etc. Other projects have various metadata lines to reference a bug-tracker id number; folks may have seen commits with various metadata id's in public git repositories such as: Google-Bug-Id: 12345
[PATCH v4 02/10] transport-helper: fix extra lines
Commit 9c51558 (transport-helper: trivial code shuffle) moved these
lines above, but 99d9ec0 (Merge branch 'fc/transport-helper-no-refspec')
had a wrong merge conflict and readded them.
Reported-by: Richard Hansen rhan...@bbn.com
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
transport-helper.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 408d596..707351d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -879,9 +879,6 @@ static int push_refs_with_export(struct transport
*transport,
}
free(private);
- if (ref-deletion)
- die(remote-helpers do not support ref deletion);
-
if (ref-peer_ref) {
if (strcmp(ref-peer_ref-name, ref-name))
die(remote-helpers do not support old:new
syntax);
--
1.8.4-fc
--
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 v4 10/10] transport-helper: don't update refs in dry-run
The remote helper namespace should not be updated.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
transport-helper.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 4f47bdd..ef91882 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -733,7 +733,8 @@ static int push_update_ref_status(struct strbuf *buf,
}
static void push_update_refs_status(struct helper_data *data,
- struct ref *remote_refs)
+ struct ref *remote_refs,
+ int flags)
{
struct strbuf buf = STRBUF_INIT;
struct ref *ref = remote_refs;
@@ -747,7 +748,7 @@ static void push_update_refs_status(struct helper_data
*data,
if (push_update_ref_status(buf, ref, remote_refs))
continue;
- if (!data-refspecs || data-no_private_update)
+ if (flags TRANSPORT_PUSH_DRY_RUN || !data-refspecs ||
data-no_private_update)
continue;
/* propagate back the update to the remote namespace */
@@ -838,7 +839,7 @@ static int push_refs_with_push(struct transport *transport,
sendline(data, buf);
strbuf_release(buf);
- push_update_refs_status(data, remote_refs);
+ push_update_refs_status(data, remote_refs, flags);
return 0;
}
@@ -905,7 +906,7 @@ static int push_refs_with_export(struct transport
*transport,
if (finish_command(exporter))
die(Error while running fast-export);
- push_update_refs_status(data, remote_refs);
+ push_update_refs_status(data, remote_refs, flags);
return 0;
}
--
1.8.4-fc
--
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 v4 03/10] transport-helper: check for 'forced update' message
So the remote-helpers can tell us when a forced push was needed.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
transport-helper.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/transport-helper.c b/transport-helper.c
index 707351d..f8eb143 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -643,7 +643,7 @@ static int push_update_ref_status(struct strbuf *buf,
struct ref *remote_refs)
{
char *refname, *msg;
- int status;
+ int status, forced = 0;
if (!prefixcmp(buf-buf, ok )) {
status = REF_STATUS_OK;
@@ -701,6 +701,11 @@ static int push_update_ref_status(struct strbuf *buf,
free(msg);
msg = NULL;
}
+ else if (!strcmp(msg, forced update)) {
+ forced = 1;
+ free(msg);
+ msg = NULL;
+ }
}
if (*ref)
@@ -722,6 +727,7 @@ static int push_update_ref_status(struct strbuf *buf,
}
(*ref)-status = status;
+ (*ref)-forced_update = forced;
(*ref)-remote_status = msg;
return !(status == REF_STATUS_OK);
}
--
1.8.4-fc
--
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 v4 08/10] fast-export: add support to delete refs
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
builtin/fast-export.c | 14 ++
t/t9350-fast-export.sh | 11 +++
2 files changed, 25 insertions(+)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index b6f623e..8ed41b4 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -673,6 +673,19 @@ static void import_marks(char *input_file)
fclose(f);
}
+static void handle_deletes(void)
+{
+ int i;
+ for (i = 0; i refspecs_nr; i++) {
+ struct refspec *refspec = refspecs[i];
+ if (*refspec-src)
+ continue;
+
+ printf(reset %s\nfrom %s\n\n,
+ refspec-dst, sha1_to_hex(null_sha1));
+ }
+}
+
int cmd_fast_export(int argc, const char **argv, const char *prefix)
{
struct rev_info revs;
@@ -762,6 +775,7 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
}
handle_tags_and_duplicates();
+ handle_deletes();
if (export_filename lastimportid != last_idnum)
export_marks(export_filename);
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index dcf..ea6c96c 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -511,4 +511,15 @@ test_expect_success 'use refspec' '
test_cmp expected actual
'
+test_expect_success 'delete refspec' '
+ git branch to-delete
+ git fast-export --refspec :refs/heads/to-delete to-delete ^to-delete
actual
+ cat expected -EOF
+ reset refs/heads/to-delete
+ from
+
+ EOF
+ test_cmp expected actual
+'
+
test_done
--
1.8.4-fc
--
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 v4 00/10] transport-helper: updates
Hi,
Here are the patches that allow transport helpers to be completely transparent;
renaming branches, deleting them, custom refspecs, --force, --dry-run,
reporting forced update, everything works.
Some of these were were sent before and rejected without a reason, but here
they are again in case anybody is interested.
This time rebased on top of the latest master, plus a few fixes.
Diff from v3:
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 9b728ca..60d4c80 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -686,7 +686,7 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
struct commit *commit;
char *export_filename = NULL, *import_filename = NULL;
uint32_t lastimportid;
- struct string_list refspecs_list;
+ struct string_list refspecs_list = STRING_LIST_INIT_NODUP;
struct option options[] = {
OPT_INTEGER(0, progress, progress,
N_(show progress after n objects)),
diff --git a/transport-helper.c b/transport-helper.c
index d94eaf4..91636d5 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -836,9 +836,6 @@ static int push_refs_with_export(struct transport
*transport,
char *private;
unsigned char sha1[20];
- if (ref-deletion)
- die(remote-helpers do not support ref deletion);
-
private = apply_refspecs(data-refspecs, data-refspec_nr,
ref-name);
if (private !get_sha1(private, sha1)) {
strbuf_addf(buf, ^%s, private);
Felipe Contreras (10):
transport-helper: add 'force' to 'export' helpers
transport-helper: fix extra lines
transport-helper: check for 'forced update' message
fast-export: improve argument parsing
fast-export: add new --refspec option
transport-helper: add support for old:new refspec
fast-import: add support to delete refs
fast-export: add support to delete refs
transport-helper: add support to delete branches
transport-helper: don't update refs in dry-run
Documentation/git-fast-export.txt | 4
Documentation/git-fast-import.txt | 3 +++
builtin/fast-export.c | 47 ++-
fast-import.c | 13 ---
t/t5801-remote-helpers.sh | 10 -
t/t9300-fast-import.sh| 18 +++
t/t9350-fast-export.sh| 18 +++
transport-helper.c| 47 ++-
8 files changed, 140 insertions(+), 20 deletions(-)
--
1.8.4-fc
--
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 v4 05/10] fast-export: add new --refspec option
So that we can convert the exported ref names.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
Documentation/git-fast-export.txt | 4
builtin/fast-export.c | 30 ++
t/t9350-fast-export.sh| 7 +++
3 files changed, 41 insertions(+)
diff --git a/Documentation/git-fast-export.txt
b/Documentation/git-fast-export.txt
index 85f1f30..221506b 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -105,6 +105,10 @@ marks the same across runs.
in the commit (as opposed to just listing the files which are
different from the commit's first parent).
+--refspec::
+ Apply the specified refspec to each ref exported. Multiple of them can
+ be specified.
+
[git-rev-list-args...]::
A list of arguments, acceptable to 'git rev-parse' and
'git rev-list', that specifies the specific objects and references
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index eea5b8c..b6f623e 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -17,6 +17,7 @@
#include utf8.h
#include parse-options.h
#include quote.h
+#include remote.h
static const char *fast_export_usage[] = {
N_(git fast-export [rev-list-opts]),
@@ -31,6 +32,8 @@ static int use_done_feature;
static int no_data;
static int full_tree;
static struct string_list extra_refs = STRING_LIST_INIT_NODUP;
+static struct refspec *refspecs;
+static int refspecs_nr;
static int parse_opt_signed_tag_mode(const struct option *opt,
const char *arg, int unset)
@@ -525,6 +528,15 @@ static void get_tags_and_duplicates(struct
rev_cmdline_info *info)
if (dwim_ref(e-name, strlen(e-name), sha1, full_name) != 1)
continue;
+ if (refspecs) {
+ char *private;
+ private = apply_refspecs(refspecs, refspecs_nr,
full_name);
+ if (private) {
+ free(full_name);
+ full_name = private;
+ }
+ }
+
commit = get_commit(e, full_name);
if (!commit) {
warning(%s: Unexpected object of type %s, skipping.,
@@ -668,6 +680,7 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
struct commit *commit;
char *export_filename = NULL, *import_filename = NULL;
uint32_t lastimportid;
+ struct string_list refspecs_list = STRING_LIST_INIT_NODUP;
struct option options[] = {
OPT_INTEGER(0, progress, progress,
N_(show progress after n objects)),
@@ -688,6 +701,8 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
OPT_BOOL(0, use-done-feature, use_done_feature,
N_(Use the done feature to terminate the
stream)),
OPT_BOOL(0, no-data, no_data, N_(Skip output of blob
data)),
+ OPT_STRING_LIST(0, refspec, refspecs_list, N_(refspec),
+N_(Apply refspec to exported refs)),
OPT_END()
};
@@ -707,6 +722,19 @@ int cmd_fast_export(int argc, const char **argv, const
char *prefix)
if (argc 1)
usage_with_options (fast_export_usage, options);
+ if (refspecs_list.nr) {
+ const char *refspecs_str[refspecs_list.nr];
+ int i;
+
+ for (i = 0; i refspecs_list.nr; i++)
+ refspecs_str[i] = refspecs_list.items[i].string;
+
+ refspecs_nr = refspecs_list.nr;
+ refspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);
+
+ string_list_clear(refspecs_list, 1);
+ }
+
if (use_done_feature)
printf(feature done\n);
@@ -741,5 +769,7 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
if (use_done_feature)
printf(done\n);
+ free_refspec(refspecs_nr, refspecs);
+
return 0;
}
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 34c2d8f..dcf 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -504,4 +504,11 @@ test_expect_success 'refs are updated even if no commits
need to be exported' '
test_cmp expected actual
'
+test_expect_success 'use refspec' '
+ git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
+ grep ^commit | sort | uniq actual
+ echo commit refs/heads/foobar expected
+ test_cmp expected actual
+'
+
test_done
--
1.8.4-fc
--
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 v4 06/10] transport-helper: add support for old:new refspec
By using fast-export's new --refspec option.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t5801-remote-helpers.sh | 2 +-
transport-helper.c| 13 ++---
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 613f69a..407c18d 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -87,7 +87,7 @@ test_expect_success 'push new branch by name' '
compare_refs local HEAD server refs/heads/new-name
'
-test_expect_failure 'push new branch with old:new refspec' '
+test_expect_success 'push new branch with old:new refspec' '
(cd local
git push origin new-name:new-refspec
)
diff --git a/transport-helper.c b/transport-helper.c
index f8eb143..5454822 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -848,7 +848,7 @@ static int push_refs_with_export(struct transport
*transport,
struct ref *ref;
struct child_process *helper, exporter;
struct helper_data *data = transport-data;
- struct string_list revlist_args = STRING_LIST_INIT_NODUP;
+ struct string_list revlist_args = STRING_LIST_INIT_DUP;
struct strbuf buf = STRBUF_INIT;
if (!data-refspecs)
@@ -886,8 +886,13 @@ static int push_refs_with_export(struct transport
*transport,
free(private);
if (ref-peer_ref) {
- if (strcmp(ref-peer_ref-name, ref-name))
- die(remote-helpers do not support old:new
syntax);
+ if (strcmp(ref-name, ref-peer_ref-name)) {
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(buf, %s:%s, ref-peer_ref-name,
ref-name);
+ string_list_append(revlist_args, --refspec);
+ string_list_append(revlist_args, buf.buf);
+ strbuf_release(buf);
+ }
string_list_append(revlist_args, ref-peer_ref-name);
}
}
@@ -895,6 +900,8 @@ static int push_refs_with_export(struct transport
*transport,
if (get_exporter(transport, exporter, revlist_args))
die(Couldn't run fast-export);
+ string_list_clear(revlist_args, 1);
+
if (finish_command(exporter))
die(Error while running fast-export);
push_update_refs_status(data, remote_refs);
--
1.8.4-fc
--
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 v4 01/10] transport-helper: add 'force' to 'export' helpers
Otherwise they cannot know when to force the push or not (other than
hacks).
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
transport-helper.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/transport-helper.c b/transport-helper.c
index b32e2d6..408d596 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -853,6 +853,11 @@ static int push_refs_with_export(struct transport
*transport,
die(helper %s does not support dry-run, data-name);
}
+ if (flags TRANSPORT_PUSH_FORCE) {
+ if (set_helper_option(transport, force, true) != 0)
+ die(helper %s does not support 'force', data-name);
+ }
+
helper = get_helper(transport);
write_constant(helper-in, export\n);
--
1.8.4-fc
--
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 v4 07/10] fast-import: add support to delete refs
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
Documentation/git-fast-import.txt | 3 +++
fast-import.c | 13 ++---
t/t9300-fast-import.sh| 18 ++
3 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-fast-import.txt
b/Documentation/git-fast-import.txt
index 73f9806..c49ede4 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -483,6 +483,9 @@ Marks must be declared (via `mark`) before they can be used.
* Any valid Git SHA-1 expression that resolves to a commit. See
``SPECIFYING REVISIONS'' in linkgit:gitrevisions[7] for details.
+* The special null SHA-1 (40 zeros) specifices that the branch is to be
+ removed.
+
The special case of restarting an incremental import from the
current branch value should be written as:
diff --git a/fast-import.c b/fast-import.c
index f4d9969..fdce0b7 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -248,6 +248,7 @@ struct branch {
uintmax_t last_commit;
uintmax_t num_notes;
unsigned active : 1;
+ unsigned delete : 1;
unsigned pack_id : PACK_ID_BITS;
unsigned char sha1[20];
};
@@ -1690,10 +1691,13 @@ static int update_branch(struct branch *b)
struct ref_lock *lock;
unsigned char old_sha1[20];
- if (is_null_sha1(b-sha1))
- return 0;
if (read_ref(b-name, old_sha1))
hashclr(old_sha1);
+ if (is_null_sha1(b-sha1)) {
+ if (b-delete)
+ delete_ref(b-name, old_sha1, 0);
+ return 0;
+ }
lock = lock_any_ref_for_update(b-name, old_sha1, 0, NULL);
if (!lock)
return error(Unable to lock %s, b-name);
@@ -2620,8 +2624,11 @@ static int parse_from(struct branch *b)
free(buf);
} else
parse_from_existing(b);
- } else if (!get_sha1(from, b-sha1))
+ } else if (!get_sha1(from, b-sha1)) {
parse_from_existing(b);
+ if (is_null_sha1(b-sha1))
+ b-delete = 1;
+ }
else
die(Invalid ref name or SHA1 expression: %s, from);
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 88fc407..f453388 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2999,4 +2999,22 @@ test_expect_success 'T: ls root tree' '
test_cmp expect actual
'
+test_expect_success 'T: delete branch' '
+ git branch to-delete
+ git fast-import -EOF
+ reset refs/heads/to-delete
+ from
+ EOF
+ test_must_fail git rev-parse --verify refs/heads/to-delete
+'
+
+test_expect_success 'T: empty reset doesnt delete branch' '
+ git branch not-to-delete
+ git fast-import -EOF
+ reset refs/heads/not-to-delete
+ EOF
+ git show-ref
+ git rev-parse --verify refs/heads/not-to-delete
+'
+
test_done
--
1.8.4-fc
--
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 v4 04/10] fast-export: improve argument parsing
We don't want to pass arguments specific to fast-export to setup_revisions. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- builtin/fast-export.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/fast-export.c b/builtin/fast-export.c index 78250ea..eea5b8c 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -701,8 +701,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix) revs.topo_order = 1; revs.show_source = 1; revs.rewrite_parents = 1; + argc = parse_options(argc, argv, prefix, options, fast_export_usage, + PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN); argc = setup_revisions(argc, argv, revs, NULL); - argc = parse_options(argc, argv, prefix, options, fast_export_usage, 0); if (argc 1) usage_with_options (fast_export_usage, options); -- 1.8.4-fc -- 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] commit: Add -f, --fixes commit option to add Fixes: line
On Sun, Oct 27, 2013 at 06:42:44AM +0100, Michael Haggerty wrote: On 10/27/2013 02:34 AM, Josh Triplett wrote: Linux Kernel Summit 2013 decided on a commit message convention to identify commits containing bugs fixed by a commit: a Fixes: line, included in the standard commit footer (along with Signed-off-by: if present), containing an abbreviated commit hash (at least 12 characters to keep it valid for a long time) and the subject of the commit (for human readers). This helps people (or automated tools) determine how far to backport a commit. Add a command line option for git commit to automatically construct the Fixes: line for a commit. This avoids the need to manually construct that line by copy-pasting the commit hash and subject. Also works with --amend to modify an existing commit's message. To add a Fixes line to an earlier commit in a series, use rebase -i and add the following line after the existing commit: x git commit --amend --no-edit -f $commit_containing_bug Generalize append_signoff to support appending arbitrary extra lines to a commit in the signoff block; this avoids duplicating the logic to find or construct that block. I have a few comments and questions about the design of this feature: First of all, let me show my ignorance. How formalized is the use of metadata lines at the end of a commit message? I don't remember seeing documentation about such lines in general (as opposed to documentation about particular types of lines). Is the format defined well enough that tools that don't know about a particular line could nonetheless preserve it correctly? Is there/should there be a standard recommended order of metadata lines? (For example, should Fixes: lines always appear before Signed-off-by lines, or vice versa?) If so, is it documented somewhere and preserved by tools when such lines are added/modified? Should there be support for querying such lines? While it isn't very well documented in git itself, metadata lines are quite standardized. See Documentation/SubmittingPatches and Documentation/development-process/5.Posting in the Linux kernel, for an explanation of Reported-by:, Tested-by:, Reviewed-by:, Suggested-by:, and Acked-by:. And git itself looks for a very specific format; the has_conforming_footer function looks for a footer consisting exclusively of rfc2822-style (email-style) header lines to decide whether to append Signed-off-by: (and now Fixes:) directly to that block or to create a new block. I do think there should be additional support for such lines in git, such as a git commit option to add Cc: lines (via a --cc-cmd like get_maintainer.pl run at commit time), or fast options in rebase -i to append arbitrary footer lines to a commit. Too bad your proposed new option sounds so similar to --fixup, which does something conceptually similar albeit very different in effect. This will likely lead to confusion. Given that the line is named Fixes:, I don't think the name of the option will extend the confusion any further. :) I wonder if the two features could be combined in some way? The main difference between the two features is how they are intended to be used: --fixup is to fix a commit that hasn't been pushed yet (where the user intends to squash the commits together), whereas --fixes is to mark a commit as a fix to a commit that has already been pushed (where the commits will remain separate). But there seems to be a common concept here. For example, what happens if a --fixes commit is rebase -ied at the same time as the commit that it fixes? It might make sense to do the autosquash thing just like with a --fixup/--squash commit. (Otherwise the SHA-1 in the Fixes: line will become invalid anyway.) Most definitely not, no, at least not without an explicit option to enable that. Consider the case of backporting a series of patches and preserving the relative history of those patches, to make it easier to match up a set of patches. At most, it might be a good idea for cherry-pick or similar to provide an updated Fixes tag for the new hash of the older commit. Personally, I'd argue against doing this even with --autosquash. I could see the argument for an --autosquash-fixes, but I can't think of a real-world scenario where what would come up. Generally, if history is still editable, you should just squash in the fix to the original commit, and if history is no longer editable (which is the use case for Fixes: lines), the squash case simply won't come up, offering little point to adding special support for that case. Conversely, I suppose one could ask whether there should be some way to prevent fixup! or squash! commits from being pushed, at least without some kind of --force option. This could of course be enforced by a hook but it might be nice to have some protection by default. That's a good idea, but unrelated to this patch. And yes, a hook seems like the
Re: [Ksummit-2013-discuss] [PATCH] commit: Add -f, --fixes commit option to add Fixes: line
On Sun, Oct 27, 2013 at 12:14 AM, Josh Triplett j...@joshtriplett.org wrote: +-f commit:: +--fixes=commit:: + Add Fixes line for the specified commit at the end of the commit + log message. This line includes an abbreviated commit hash for + the specified commit; the `core.abbrev` option determines the + length of the abbreviated commit hash used, with a minimum length + of 12 hex digits. You might also mention that the Fixes: line includes the old commit's subject line. I only mentioned the abbreviated commit hash because it was necessary to explain the factors affecting hash length. -s, above, doesn't mention that the Signed-off-by line includes the name and email address of the committer. I do wonder, if we're going to bake into git the idea that too-short abbreviated sha1s don't make sense, why don't we just change the core.abbrev default to 12 everywhere rather than just in this one command ? -- Michel Walken Lespinasse A program is never fully debugged until the last user dies. -- 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] commit: Add -f, --fixes commit option to add Fixes: line
Josh Triplett j...@joshtriplett.org writes: On Sun, Oct 27, 2013 at 06:42:44AM +0100, Michael Haggerty wrote: But I don't think that this feature should be given the -f short option, as (a) -f often means force; (b) it will increase the confusion with --fixup; (c) it just doesn't strike me as being likely to be such a frequently-used option (though if this changes over time the -f option could always be granted to it later). (a) -n often means --dry-run, but for commit it means --no-verify. Different commands have different options, and commit doesn't have a --force to abbreviate as -f. (b) If anything, I think the existence of a short option will make the distinction more obvious, since -f and --fixup are much less similar than --fixes and --fixup. Most users will never type --fixes, making confusion unlikely. (c) Short option letters tend to be first-come first-serve unless there's a strong reason to do otherwise. Why reserve 'f' for some hypothetical future option that doesn't exist yet? No, lately the direction in Git has been to avoid giving options a one-letter shorthand until they have proven so useful that people using it in the wild start to suggest that it should have one. See e.g. http://article.gmane.org/gmane.comp.version-control.git/233998 http://article.gmane.org/gmane.comp.version-control.git/168748 A much better argument would be if it was already clear from the specs laid out for Fixes that n% of the kernel commits will end up having this footer, and thus kernel hackers will spend x amount of time spelling out --fixes and/or confusing it with --fixup to much headache. -- Thomas Rast t...@thomasrast.ch -- 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] commit: Add -f, --fixes commit option to add Fixes: line
On Sun, Oct 27, 2013 at 8:34 AM, Josh Triplett j...@joshtriplett.org wrote:
Add a command line option for git commit to automatically construct the
Fixes: line for a commit. This avoids the need to manually construct
that line by copy-pasting the commit hash and subject.
But you still have to copy/paste the hash in command line. I wonder if
we should approach it differently: the user writes Fixes: hash in
the commit message, then git detects these lines and expands them
using a user-configured format. For the kernel circle, the format
would be %h ('%s') (I'll need to think how to let the user say
minimum 12 chars).
Other projects need to refer to old commits sometimes in commit
messages too and this could be extended further to expand inline
abbrev sha-1s, but to not break the text alignment badly, maybe
footnotes will be created to store subjects and stuff, rather than do
inline expansion. For example,
commit 1232343 breaks something.
becomes
comit 1232343 [1] breaks something
[1] 123234332131 (do something wrong - at this date)
--
Duy
--
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] commit: Add -f, --fixes commit option to add Fixes: line
On Sun, Oct 27, 2013 at 03:33:19PM +0700, Duy Nguyen wrote:
On Sun, Oct 27, 2013 at 8:34 AM, Josh Triplett j...@joshtriplett.org wrote:
Add a command line option for git commit to automatically construct the
Fixes: line for a commit. This avoids the need to manually construct
that line by copy-pasting the commit hash and subject.
But you still have to copy/paste the hash in command line. I wonder if
we should approach it differently: the user writes Fixes: hash in
the commit message, then git detects these lines and expands them
Then you have to copy/paste the hash into the commit message; either way
you're not getting around that. However, note that you can pass a ref
instead of a commit hash, if you happen to have saved a tag pointing to
the broken ref. (Or, for instance, if you have it from a bisection.)
I could imagine supporting that approach in addition (via a commit-msg
hook, for instance), but I'd still like to have the command-line option
to git commit.
using a user-configured format. For the kernel circle, the format
would be %h ('%s') (I'll need to think how to let the user say
minimum 12 chars).
I considered making the format configurable, and that's easy enough to
do, but I wanted to start out with the simplest patch that achieved the
goal, on the theory that it's easy to add configurability later if
anyone actually needs it.
Other projects need to refer to old commits sometimes in commit
messages too and this could be extended further to expand inline
abbrev sha-1s, but to not break the text alignment badly, maybe
footnotes will be created to store subjects and stuff, rather than do
inline expansion. For example,
commit 1232343 breaks something.
becomes
comit 1232343 [1] breaks something
[1] 123234332131 (do something wrong - at this date)
Easily done via a commit-msg hook, if you want that.
- Josh Triplett
--
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] commit: Add -f, --fixes commit option to add Fixes: line
On Sun, Oct 27, 2013 at 09:09:32AM +0100, Thomas Rast wrote: Josh Triplett j...@joshtriplett.org writes: On Sun, Oct 27, 2013 at 06:42:44AM +0100, Michael Haggerty wrote: But I don't think that this feature should be given the -f short option, as (a) -f often means force; (b) it will increase the confusion with --fixup; (c) it just doesn't strike me as being likely to be such a frequently-used option (though if this changes over time the -f option could always be granted to it later). (a) -n often means --dry-run, but for commit it means --no-verify. Different commands have different options, and commit doesn't have a --force to abbreviate as -f. (b) If anything, I think the existence of a short option will make the distinction more obvious, since -f and --fixup are much less similar than --fixes and --fixup. Most users will never type --fixes, making confusion unlikely. (c) Short option letters tend to be first-come first-serve unless there's a strong reason to do otherwise. Why reserve 'f' for some hypothetical future option that doesn't exist yet? No, lately the direction in Git has been to avoid giving options a one-letter shorthand until they have proven so useful that people using it in the wild start to suggest that it should have one. See e.g. http://article.gmane.org/gmane.comp.version-control.git/233998 http://article.gmane.org/gmane.comp.version-control.git/168748 Fair enough; easy enough to drop -f if that's the consensus. However... A much better argument would be if it was already clear from the specs laid out for Fixes that n% of the kernel commits will end up having this footer, and thus kernel hackers will spend x amount of time spelling out --fixes and/or confusing it with --fixup to much headache. ...good suggestion: ~/src/linux$ git log --grep='stable@' --oneline --since='1 year ago' | wc -l 2769 ~/src/linux$ git log --grep='stable@' --oneline --since='1 year ago' --pretty=format:%an | sort -u | wc -l 839 Several thousand commits per year by hundreds of unique people seems like enough to justify a short option. - Josh Triplett -- 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: [Ksummit-2013-discuss] [PATCH] commit: Add -f, --fixes commit option to add Fixes: line
On Sun, Oct 27, 2013 at 01:03:47AM -0700, Michel Lespinasse wrote: On Sun, Oct 27, 2013 at 12:14 AM, Josh Triplett j...@joshtriplett.org wrote: +-f commit:: +--fixes=commit:: + Add Fixes line for the specified commit at the end of the commit + log message. This line includes an abbreviated commit hash for + the specified commit; the `core.abbrev` option determines the + length of the abbreviated commit hash used, with a minimum length + of 12 hex digits. You might also mention that the Fixes: line includes the old commit's subject line. I only mentioned the abbreviated commit hash because it was necessary to explain the factors affecting hash length. -s, above, doesn't mention that the Signed-off-by line includes the name and email address of the committer. I do wonder, if we're going to bake into git the idea that too-short abbreviated sha1s don't make sense, why don't we just change the core.abbrev default to 12 everywhere rather than just in this one command ? You won't get any argument from me on that one. I personally would have argued for making the hashes 40 characters always, but in any case bumping up the default (and minimum) for core.abbrev seems entirely sensible. - Josh Triplett -- 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] commit: Add -f, --fixes commit option to add Fixes: line
On 10/27/2013 09:09 AM, Thomas Rast wrote: Josh Triplett j...@joshtriplett.org writes: On Sun, Oct 27, 2013 at 06:42:44AM +0100, Michael Haggerty wrote: But I don't think that this feature should be given the -f short option, as (a) -f often means force; (b) it will increase the confusion with --fixup; (c) it just doesn't strike me as being likely to be such a frequently-used option (though if this changes over time the -f option could always be granted to it later). (a) -n often means --dry-run, but for commit it means --no-verify. Different commands have different options, and commit doesn't have a --force to abbreviate as -f. (b) If anything, I think the existence of a short option will make the distinction more obvious, since -f and --fixup are much less similar than --fixes and --fixup. Most users will never type --fixes, making confusion unlikely. (c) Short option letters tend to be first-come first-serve unless there's a strong reason to do otherwise. Why reserve 'f' for some hypothetical future option that doesn't exist yet? No, lately the direction in Git has been to avoid giving options a one-letter shorthand until they have proven so useful that people using it in the wild start to suggest that it should have one. See e.g. http://article.gmane.org/gmane.comp.version-control.git/233998 http://article.gmane.org/gmane.comp.version-control.git/168748 A much better argument would be if it was already clear from the specs laid out for Fixes that n% of the kernel commits will end up having this footer, and thus kernel hackers will spend x amount of time spelling out --fixes and/or confusing it with --fixup to much headache. I assembled an overview table, which plots the long options of git commands by the short letters. Here it is: (Best viewed with a *large* screen and monospace font) Name\short | C | B | A | G | F |E | H | O | N |L | S |R | P | W |X | c | b | a | g | f |e |d | k |i | o | n | m | l | s |r | q | p |w | v | u | t | z |x | 3 | 2 status || || || | | || | | | | | | | branch | | || | | | | | | | | short | ||| | verbose | untracked-files | | null | | | status help || || || | | || | | | | | | | | all | guides || | | | info | | | man | | | ||| web | | | | | | | help show || || || | | || | | | | | | | | | || | | | | | | | | | | quiet || | | | | | | | show revert || || || | | || | | | | | strategy-option | | | | || edit | | | | | no-commit | mainline | | signoff | ||| | | | | |
git stash does not work when directory is replaced by a symlink to itself
Hi, list! See below. Best, Alexander. agladysh@work-1:~/tmp/git$ git --version git version 1.8.4.1 agladysh@work-1:~/tmp/git$ cat ./test.sh #!/bin/bash set -e mkdir alpha cd alpha git init mkdir beta echo gamma beta/gamma git add beta/gamma git commit -m initial commit mv beta delta ln -s delta beta git stash agladysh@work-1:~/tmp/git$ ./test.sh Initialized empty Git repository in /home/agladysh/tmp/git/alpha/.git/ [master (root-commit) 9c6138f] initial commit 1 file changed, 1 insertion(+) create mode 100644 beta/gamma error: 'beta/gamma' is beyond a symbolic link fatal: Unable to process path beta/gamma Cannot save the current worktree state agladysh@work-1:~/tmp/git$ lsb_release -a No LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 12.10 Release: 12.10 Codename: quantal agladysh@work-1:~/tmp/git$ uname -a Linux work-1 3.5.0-34-generic #55-Ubuntu SMP Thu Jun 6 20:18:19 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux -- 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 v4 07/10] fast-import: add support to delete refs
On Sun, Oct 27, 2013 at 3:05 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/git-fast-import.txt | 3 +++ fast-import.c | 13 ++--- t/t9300-fast-import.sh| 18 ++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index 73f9806..c49ede4 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -483,6 +483,9 @@ Marks must be declared (via `mark`) before they can be used. * Any valid Git SHA-1 expression that resolves to a commit. See ``SPECIFYING REVISIONS'' in linkgit:gitrevisions[7] for details. +* The special null SHA-1 (40 zeros) specifices that the branch is to be s/specifices/specifies/ + removed. + The special case of restarting an incremental import from the current branch value should be written as: -- 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] commit: Add -f, --fixes commit option to add Fixes: line
On Sun, Oct 27, 2013 at 10:20 AM, Josh Triplett j...@joshtriplett.org wrote: On Sun, Oct 27, 2013 at 09:09:32AM +0100, Thomas Rast wrote: Josh Triplett j...@joshtriplett.org writes: On Sun, Oct 27, 2013 at 06:42:44AM +0100, Michael Haggerty wrote: But I don't think that this feature should be given the -f short option, as (a) -f often means force; (b) it will increase the confusion with --fixup; (c) it just doesn't strike me as being likely to be such a frequently-used option (though if this changes over time the -f option could always be granted to it later). (a) -n often means --dry-run, but for commit it means --no-verify. Different commands have different options, and commit doesn't have a --force to abbreviate as -f. (b) If anything, I think the existence of a short option will make the distinction more obvious, since -f and --fixup are much less similar than --fixes and --fixup. Most users will never type --fixes, making confusion unlikely. (c) Short option letters tend to be first-come first-serve unless there's a strong reason to do otherwise. Why reserve 'f' for some hypothetical future option that doesn't exist yet? No, lately the direction in Git has been to avoid giving options a one-letter shorthand until they have proven so useful that people using it in the wild start to suggest that it should have one. See e.g. http://article.gmane.org/gmane.comp.version-control.git/233998 http://article.gmane.org/gmane.comp.version-control.git/168748 Fair enough; easy enough to drop -f if that's the consensus. However... A much better argument would be if it was already clear from the specs laid out for Fixes that n% of the kernel commits will end up having this footer, and thus kernel hackers will spend x amount of time spelling out --fixes and/or confusing it with --fixup to much headache. ...good suggestion: ~/src/linux$ git log --grep='stable@' --oneline --since='1 year ago' | wc -l 2769 ~/src/linux$ git log --grep='stable@' --oneline --since='1 year ago' --pretty=format:%an | sort -u | wc -l 839 Several thousand commits per year by hundreds of unique people seems like enough to justify a short option. I think this can be solved just as well (if not better) using a combination of a commit message template (or a prepare-commit-msg hook) and a commit-msg hook. The former appends a section of commonly-used RFC822-style headers (with empty values) to the bottom of the commit message, e.g. some variation on this: Fixes: Reported-by: Suggested-by: Improved-by: Acked-by: Reviewed-by: Tested-by: Signed-off-by: Then the user (in addition to writing the commit message above this block) may choose to fill in one or more values in this form, e.g. like this: My commit subject This is the commit message body. Fixes: 1234beef Reported-by: Joe User j.u...@example.com Suggested-by: Improved-by: Joe Hacker j.hac...@example.com Acked-by: Reviewed-by: Tested-by: Joe Tester j.tes...@example.com Signed-off-by: Myself mys...@example.com Then, the commit-msg hook can clean up and transform this into the final commit message: My commit subject This is the commit message body. Fixes: 1234beef56 (Commit message summmary) Reported-by: Joe User j.u...@example.com Improved-by: Joe Hacker j.hac...@example.com Tested-by: Joe Tester j.tes...@example.com Signed-off-by: Myself mys...@example.com Here, the commit-msg hook removes the fields that were not filled in, and performs additional filtering on the Fixes line (Adding commit message summary). The filtering could also resolve ref names, so that if you had refs/tags/security-bug pointing at the buggy commit, then: Fixes: security-bug would be expanded/DWIMed into: Fixes: 1234beef56 (Commit message summmary) Obviously, any other fancy processing you want to do into in the commit-msg hook can be done as well, adding footnotes, checking that commits are present in the ancestry, etc, etc. Three good reasons to go this way: 1. If the user forgets to supply command-line options like -s, --fixes, etc, there is a nice reminder in the supplied form. 2. No need to add any command-line options to Git. 3. The whole mechanism is controlled by the project. The kernel folks can do whatever they want in their templates/hooks without needing changes to the Git project. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- 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] sha1_file.c:create_tmpfile(): Fix race when creating loose object dirs
There are cases (e.g. when running concurrent fetches in a repo) where multiple Git processes concurrently attempt to create loose objects within the same objects/XX/ dir. The creation of the loose object files is (AFAICS) safe from races, but the creation of the objects/XX/ dir in which the loose objects reside is unsafe, for example: Two concurrent fetches - A and B. As part of its fetch, A needs to store 12a as a loose object. B, on the other hand, needs to store 12b as a loose object. The objects/12 directory does not already exist. Concurrently, both A and B determine that they need to create the objects/12 directory (because their first call to git_mkstemp_mode() within create_tmpfile() fails witn ENOENT). One of them - let's say A - executes the following mkdir() call before the other. This first call returns success, and A moves on. When B gets around to calling mkdir(), it fails with EEXIST, because A won the race. The mkdir() error causes B to return -1 from create_tmpfile(), which propagates all the way, resulting in the fetch failing with: error: unable to create temporary file: File exists fatal: failed to write object fatal: unpack-objects failed Although it's hard to add a testcase reproducing this issue, it's easy to provoke if we insert a sleep after the if (mkdir(buffer, 0777) || adjust_shared_perm(buffer)) return -1; block, and then run two concurrent git fetches against the same repo. The fix is to simply handle mkdir() failing with EEXIST as a success. If EEXIST is somehow returned for the wrong reasons (because the relevant objects/XX is not a directory, or is otherwise unsuitable for object storage), the following call to adjust_shared_perm(), or ultimately the retried call to git_mkstemp_mode() will fail, and we end up returning error from create_tmpfile() in any case. Note that there are still cases where two users with unsuitable umasks in a shared repo can end up in two races where one user first wins the mkdir() race to create an objects/XX/ directory, and then the other user wins the adjust_shared_perms() race to chmod() that directory, but fails because it is (transiently, until the first users completes its chmod()) unwriteable to the other user. However, (an equivalent of) this race also exists before this patch, and is made no worse by this patch. Signed-off-by: Johan Herland jo...@herland.net --- I didn't see this in the latest What's cooking, so here's a resend, with an expanded commit message to reflect our discussion. The patch itself is unchanged. In order to fix the remaining race, I assume we have to ensure the dir creation obeys the same rules as the object creation, i.e. that there are only two possible states at any time: - The directory does not exist - The directory exists with the correct permissons To achieve this, I guess we have to follow the same procedure we do for loose object creation: 1. Create a temporary directory with a unique name (mkdtemp?) 2. Adjust permissions 3. Rename into place Can this be done sufficiently atomically across all platforms? ...Johan sha1_file.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sha1_file.c b/sha1_file.c index f80bbe4..00e 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2857,7 +2857,9 @@ static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename) /* Make sure the directory exists */ memcpy(buffer, filename, dirlen); buffer[dirlen-1] = 0; - if (mkdir(buffer, 0777) || adjust_shared_perm(buffer)) + if (mkdir(buffer, 0777) errno != EEXIST) + return -1; + if (adjust_shared_perm(buffer)) return -1; /* Try again */ -- 1.8.4.653.g2df02b3 -- 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] reflog: handle lightweight and annotated tags equally
When 'git reflog tag' is called on a lightweight tag, nothing is
output. However, when called on an annotated tag, shortened SHA-1s for
all reachable commits are written on a single line.
Teach add_pending_object_with_mode() to handle OBJ_TAG, so that 'git
reflog' on an annotated tag is quiet, like it is for lightweight tags,
commits and blobs.
Signed-off-by: Torstein Hegge he...@resisty.net
---
Try 'GIT_PAGER=cat git reflog v1.8.4' on git.git to see an example of
this rather unexpected behavior.
revision.c | 2 +-
t/t1411-reflog-show.sh | 28
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/revision.c b/revision.c
index 0173e01..fa4e660 100644
--- a/revision.c
+++ b/revision.c
@@ -198,7 +198,7 @@ static void add_pending_object_with_mode(struct rev_info
*revs,
return;
if (revs-no_walk (obj-flags UNINTERESTING))
revs-no_walk = 0;
- if (revs-reflog_info obj-type == OBJ_COMMIT) {
+ if (revs-reflog_info) {
struct strbuf buf = STRBUF_INIT;
int len = interpret_branch_name(name, 0, buf);
int st;
diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh
index 6f47c0d..de9fb8d 100755
--- a/t/t1411-reflog-show.sh
+++ b/t/t1411-reflog-show.sh
@@ -166,4 +166,32 @@ test_expect_success 'git log -g -p shows diffs vs.
parents' '
test_cmp expect actual
'
+test_expect_success 'add annotated tag' '
+ git tag -a -m tag message annotated-tag
+'
+
+: expect
+test_expect_success 'reflog on a tag' '
+ git reflog two actual
+ test_cmp expect actual
+'
+
+: expect
+test_expect_success 'reflog on an annotated tag' '
+ git reflog annotated-tag actual
+ test_cmp expect actual
+'
+
+: expect
+test_expect_success 'log -g on a tag' '
+ git log -g two actual
+ test_cmp expect actual
+'
+
+: expect
+test_expect_success 'log -g on an annotated tag' '
+ git log -g annotated-tag actual
+ test_cmp expect actual
+'
+
test_done
--
1.8.4.1.808.g053d237
--
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] test-lib: fix typo in comment
Point test writers to the test_expect_* functions properly.
Signed-off-by: Torstein Hegge he...@resisty.net
---
t/test-lib.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0fa7dfd..3dc1792 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -325,7 +325,7 @@ trap 'die' EXIT
. $TEST_DIRECTORY/test-lib-functions.sh
# You are not expected to call test_ok_ and test_failure_ directly, use
-# the text_expect_* functions instead.
+# the test_expect_* functions instead.
test_ok_ () {
test_success=$(($test_success + 1))
--
1.8.4.1.808.g053d237
--
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] commit: Add -f, --fixes commit option to add Fixes: line
Stefan Beller stefanbel...@googlemail.com writes: I assembled an overview table, which plots the long options of git commands by the short letters. [...] (In case thunderbird messes it up, here it is again http://pastebin.com/raw.php?i=JBci2Krx) As you can see, f is always --force except for git-config, where it is --file Woah! Impressive work. Did you autogenerate this? If so, can we have it as a small make target somewhere? If not, can you send a patch to put your table in Documentation somewhere? -- Thomas Rast t...@thomasrast.ch -- 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] commit: Add -f, --fixes commit option to add Fixes: line
On 10/27/2013 05:30 PM, Thomas Rast wrote:
Stefan Beller stefanbel...@googlemail.com writes:
I assembled an overview table, which plots the long options of
git commands by the short letters.
[...]
(In case thunderbird messes it up, here it is again
http://pastebin.com/raw.php?i=JBci2Krx)
As you can see, f is always --force except for git-config, where it is --file
Woah! Impressive work. Did you autogenerate this? If so, can we have
it as a small make target somewhere? If not, can you send a patch to
put your table in Documentation somewhere?
I thought about generating it by parsing the man pages,
but I felt it would not be reliable enough and quite time consuming
to come up with a parser. Parsing the C sources however also seemed time
consuming,
so I decided to come up with this patch:
--8--
Subject: [PATCH] parse-options: print all options having short and long form
and exit
This patch basically only prints all options which have a long and a short form
and then aborts the program. A typical output looks like this:
./git-add
add, n, dry-run
add, v, verbose
add, i, interactive
add, p, patch
add, e, edit
add, f, force
add, u, update
add, N, intent-to-add
add, A, all
---
parse-options.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/parse-options.c b/parse-options.c
index 62e9b1c..b356ca9 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -500,6 +500,12 @@ int parse_options(int argc, const char **argv, const char
*prefix,
{
struct parse_opt_ctx_t ctx;
+ for (; options-type != OPTION_END; options++) {
+ if (options-long_name options-short_name)
+ printf(%s, %c, %s\n, argv[0], options-short_name,
options-long_name);
+ }
+ exit(1);
+
parse_options_start(ctx, argc, argv, prefix, options, flags);
switch (parse_options_step(ctx, options, usagestr)) {
case PARSE_OPT_HELP:
--
1.8.4.1.605.g23c6912
Unfortunately we can only check git commands, which are written in C.
You'll notice all the perl/shell written commands are missing (rebase, etc).
Also a few commands written in C cannot easily be picked up, as they do stuff
before calling parse_options. [typically something like if (argc != 4)
print_usage();]
These commands are also not contained.
The generation of the table however was just a little python:
--8--
#!/usr/bin/python
cmds=git-add
git-apply
git-archive
git-branch
git-check-attr
git-check-ignore
git-check-mailmap
git-checkout
git-checkout-index
git-cherry
git-cherry-pick
git-clean
git-clone
git-column
git-commit
git-config
git-count-objects
git-credential-cache
git-credential-store
git-describe
git-fetch
git-fmt-merge-msg
git-for-each-ref
git-format-patch
git-fsck
git-fsck-objects
git-gc
git-grep
git-hash-object
git-help
git-init
git-init-db
git-log
git-ls-files
git-ls-tree
git-merge
git-merge-base
git-merge-file
git-merge-ours
git-mktree
git-mv
git-name-rev
git-notes
git-pack-objects
git-pack-refs
git-prune
git-prune-packed
git-push
git-read-tree
git-reflog
git-remote
git-repack
git-replace
git-rerere
git-reset
git-revert
git-rev-parse
git-rm
git-show
git-show-branch
git-show-ref
git-stage
git-status
git-symbolic-ref
git-tag
git-update-index
git-update-ref
git-update-server-info
git-verify-pack
git-verify-tag
git-whatchanged
git-write-tree
import subprocess
shorts={}
cmdoptions={}
for cmd in cmds.split(\n):
p = subprocess.Popen(./+cmd, stdout=subprocess.PIPE)
p.wait()
lines = p.stdout.read()
for line in lines.split(\n):
if not len(line):
continue
name, short, long = line.split(,)
if not short in shorts:
shorts[short] = len(long)
else:
shorts[short] = max(shorts[short], len(long))
if not name in cmdoptions:
cmdoptions[name] = {}
cmdoptions[name][short] = long
longest_cmd = 0
for cmd in cmdoptions:
longest_cmd = max(longest_cmd, len(cmd))
print *(longest_cmd-len(Name\\short)), Name\\short,
for short in shorts:
print | + *(1+shorts[short]-len(short)) + short,
print
for cmd in cmdoptions:
print *(longest_cmd-len(cmd)), cmd,
for short in shorts:
s =
if short in cmdoptions[cmd]:
s = cmdoptions[cmd][short]
print | + *(1+shorts[short]-len(s)) + s,
print , cmd
--8--
I am not sure if we should add such code to the git code base, as it would need
some cleanup.
The existing table however would become outdated fast?
So I do not have a good idea, how such a table could be easily incorporated and
kept up to date.
Thanks,
Stefan
--
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/RFC] git clone: is an URL local or ssh
On Saturday, October 26, 2013, Torsten Bögershausen wrote:
diff --git a/connect.c b/connect.c
index 06e88b0..903063e 100644
--- a/connect.c
+++ b/connect.c
@@ -564,9 +574,9 @@ struct child_process *git_connect(int fd[2], const char
*url_orig,
char *url;
char *host, *path;
char *end;
- int c;
+ int seperator;
s/seperator/separator/g
struct child_process *conn = no_fork;
- enum protocol protocol = PROTO_LOCAL;
+ enum protocol protocol = PROTO_LOCAL_OR_SSH;
int free_path = 0;
char *port = NULL;
const char **arg;
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 1d1c875..69af007 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -294,39 +294,93 @@ test_expect_success 'setup ssh wrapper' '
export TRASH_DIRECTORY
'
-clear_ssh () {
- $TRASH_DIRECTORY/ssh-output
-}
-
-expect_ssh () {
+i6501=0
Is this variable meant to be named after the test script t5601? If so:
s/i6501/i5601/g
+# $1 url
+# $2 none|host
+# $3 path
+test_clone_url () {
+ i6501=$(($i6501 + 1))
+ $TRASH_DIRECTORY/ssh-output
+ test_might_fail git clone $1 tmp$i6501
{
- case $1 in
+ case $2 in
none)
;;
*)
- echo ssh: $1 git-upload-pack '$2'
+ echo ssh: $2 git-upload-pack '$3'
esac
} $TRASH_DIRECTORY/ssh-expect
- (cd $TRASH_DIRECTORY test_cmp ssh-expect ssh-output)
+ (cd $TRASH_DIRECTORY test_cmp ssh-expect ssh-output) {
+ rm -rf ssh-expect ssh-output
+ }
Should the 'rm' be inside the (cd...) subshell? If not, are the braces
wrapping 'rm' needed, and wouldn't you want to prefix the paths with
$TRASH_DIRECTORY/?
}
-test_expect_success 'cloning myhost:src uses ssh' '
- clear_ssh
- git clone myhost:src ssh-clone
- expect_ssh myhost src
-'
--
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
Loan Offer
We offer Loan for 3% if you are interested do send details.contact Mr Tony Bowyer: coastalfinanceloanf...@hotmail.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
Re: [PATCH] commit: Add -f, --fixes commit option to add Fixes: line
[Sorry I already sent the reply below to Johan only instead of everyone.] Hi Johan, On Sun, Oct 27, 2013 at 11:59 AM, Johan Herland jo...@herland.net wrote: On Sun, Oct 27, 2013 at 10:20 AM, Josh Triplett j...@joshtriplett.org wrote: ...good suggestion: ~/src/linux$ git log --grep='stable@' --oneline --since='1 year ago' | wc -l 2769 ~/src/linux$ git log --grep='stable@' --oneline --since='1 year ago' --pretty=format:%an | sort -u | wc -l 839 Several thousand commits per year by hundreds of unique people seems like enough to justify a short option. I think this can be solved just as well (if not better) using a combination of a commit message template (or a prepare-commit-msg hook) and a commit-msg hook. Your suggestion is very good, and it is not incompatible with command line options. So both could be implemented and even work together. For example if -f ack:Peff was passed to the command line, git commit could lookup in the commit message template and see if there is one RFC822-style header that starts with or contains ack (discarding case) and it could look in some previous commits if there is an author whose name contains Peff (discarding case) and if it is the case it could append the following to the bottom of the commit message: Fixes: Reported-by: Suggested-by: Improved-by: Acked-by: Jeff King p...@peff.net Reviewed-by: Tested-by: Signed-off-by: Myself mys...@example.com (I suppose that the sob is automatically added.) It would work also with -f fix:security-bug and would put something like what you suggested: Fixes: 1234beef56 (Commit message summmary) Then, the commit-msg hook can clean up and transform this into the final commit message: My commit subject This is the commit message body. Fixes: 1234beef56 (Commit message summmary) Reported-by: Joe User j.u...@example.com Improved-by: Joe Hacker j.hac...@example.com Tested-by: Joe Tester j.tes...@example.com Signed-off-by: Myself mys...@example.com Here, the commit-msg hook removes the fields that were not filled in, and performs additional filtering on the Fixes line (Adding commit message summary). The filtering could also resolve ref names, so that if you had refs/tags/security-bug pointing at the buggy commit, then: Fixes: security-bug would be expanded/DWIMed into: Fixes: 1234beef56 (Commit message summmary) Obviously, any other fancy processing you want to do into in the commit-msg hook can be done as well, adding footnotes, checking that commits are present in the ancestry, etc, etc. Yeah, the commit message hook could do some more processing if the user adds or changes stuff. Three good reasons to go this way: 1. If the user forgets to supply command-line options like -s, --fixes, etc, there is a nice reminder in the supplied form. Great! 2. No need to add any command-line options to Git. This is not a good reason. If many users prefer a command line option, why not let them use that? 3. The whole mechanism is controlled by the project. The kernel folks can do whatever they want in their templates/hooks without needing changes to the Git project. The Git project already manages sob lines. It would be a good thing if it could manage more of this stuff to help users in a generic way while taking care of user preferences. Best regards, 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
git-completion.bash error
Hi. First of all, thank you and congratulations for the awesome work :) I've just made a clean install of OS X Mavericks and installed Git via Homebrew, which obviously includes the contrib files. I sourced the git-completion.bash into my profile and I get stuck on a error every time I try to use the autocomplete. The error outputted is: egrep: ^ [a-zA-Z0-9]: No such file or directory I believe this is related to this line (this commands outputs the same error when entered directly): https://github.com/git/git/blob/master/contrib/completion/git-completion.bash#L614 I don't know if this issue is directly related to OS X Mavericks or anything else. 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 v4 11/10] fixup! transport-helper: add 'force' to 'export' helpers
document the new 'force' option
Signed-off-by: Richard Hansen rhan...@bbn.com
---
Documentation/gitremote-helpers.txt | 4
1 file changed, 4 insertions(+)
diff --git a/Documentation/gitremote-helpers.txt
b/Documentation/gitremote-helpers.txt
index f1f4ca9..e75699c 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -437,6 +437,10 @@ set by Git if the remote helper has the 'option'
capability.
'option check-connectivity' \{'true'|'false'\}::
Request the helper to check connectivity of a clone.
+'option force' \{'true'|'false'\}::
+ Request the helper to perform a force update. Defaults to
+ 'false'.
+
SEE ALSO
linkgit:git-remote[1]
--
1.8.4.1.610.g2fe5c0e
--
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 v4 12/10] git-remote-testgit: support the new 'force' option
Signed-off-by: Richard Hansen rhan...@bbn.com
---
git-remote-testgit.sh | 18 ++
1 file changed, 18 insertions(+)
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 6d2f282..80546c1 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -6,6 +6,7 @@ url=$2
dir=$GIT_DIR/testgit/$alias
prefix=refs/testgit/$alias
+forcearg=
default_refspec=refs/heads/*:${prefix}/heads/*
@@ -39,6 +40,7 @@ do
fi
test -n $GIT_REMOTE_TESTGIT_SIGNED_TAGS echo signed-tags
test -n $GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE echo
no-private-update
+ echo 'option'
echo
;;
list)
@@ -93,6 +95,7 @@ do
before=$(git for-each-ref --format=' %(refname) %(objectname) ')
git fast-import \
+ ${forcearg} \
${testgitmarks:+--import-marks=$testgitmarks} \
${testgitmarks:+--export-marks=$testgitmarks} \
--quiet
@@ -115,6 +118,21 @@ do
echo
;;
+ option\ *)
+ read cmd opt val EOF
+${line}
+EOF
+ case ${opt} in
+ force)
+ case ${val} in
+ true) forcearg=--force; echo 'ok';;
+ false) forcearg=; echo 'ok';;
+ *) printf %s\\n error '${val}'\
+ is not a valid value for option ${opt};;
+ esac;;
+ *) echo unsupported;;
+ esac
+ ;;
'')
exit
;;
--
1.8.4.1.614.ga09cf56
--
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 v4 13/10] test: remote-helper: add test for force pushes
Signed-off-by: Richard Hansen rhan...@bbn.com
---
t/t5801-remote-helpers.sh | 13 +
1 file changed, 13 insertions(+)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index be543c0..93a7d34 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -102,6 +102,19 @@ test_expect_success 'push delete branch' '
rev-parse --verify refs/heads/new-name
'
+test_expect_success 'force push' '
+ (cd local
+git checkout -b force-test
+echo content file
+git commit -a -m eight
+git push origin force-test
+echo content file
+git commit -a --amend -m eight-modified
+git push --force origin force-test
+ )
+ compare_refs local refs/heads/force-test server refs/heads/force-test
+'
+
test_expect_success 'cloning without refspec' '
GIT_REMOTE_TESTGIT_REFSPEC= \
git clone testgit::${PWD}/server local2 2error
--
1.8.4.1.614.ga09cf56
--
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] Change sed i\ usage to something Solaris' sed can handle
Solaris' sed was choking on the i\ commands used in t4015-diff-whitespace as it couldn't parse the program properly. Modify two uses of sed that worked in GNU sed but not Solaris' (/usr/bin or /usr/xpg4/bin) to an equivalent form that is handled properly by both. Signed-off-by: Ben Walton bdwal...@gmail.com --- t/t4015-diff-whitespace.sh | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh index 3fb4b97..0126154 100755 --- a/t/t4015-diff-whitespace.sh +++ b/t/t4015-diff-whitespace.sh @@ -145,7 +145,8 @@ test_expect_success 'another test, with --ignore-space-at-eol' 'test_cmp expect test_expect_success 'ignore-blank-lines: only new lines' ' test_seq 5 x git update-index x - test_seq 5 | sed /3/i \\ + test_seq 5 | sed /3/i\\ +\ x git diff --ignore-blank-lines out expect @@ -155,7 +156,8 @@ test_expect_success 'ignore-blank-lines: only new lines' ' test_expect_success 'ignore-blank-lines: only new lines with space' ' test_seq 5 x git update-index x - test_seq 5 | sed /3/i \ x + test_seq 5 | sed /3/i\\ + x git diff -w --ignore-blank-lines out expect test_cmp out expect -- 1.8.1.2 -- 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: git-completion.bash error
On Sun, Oct 27, 2013 at 4:59 PM, Gabriel gabriel...@gmail.com wrote: I've just made a clean install of OS X Mavericks and installed Git via Homebrew, which obviously includes the contrib files. I sourced the git-completion.bash into my profile and I get stuck on a error every time I try to use the autocomplete. The error outputted is: egrep: ^ [a-zA-Z0-9]: No such file or directory I believe this is related to this line (this commands outputs the same error when entered directly): https://github.com/git/git/blob/master/contrib/completion/git-completion.bash#L614 Unable to reproduce on Mavericks either via the completion script or manual entry. I don't know if this issue is directly related to OS X Mavericks or anything else. -- 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] commit: Add -f, --fixes commit option to add Fixes: line
On 10/26/13 18:34, Josh Triplett wrote:
Linux Kernel ... Fixes: line ... containing an abbreviated commit hash
!-- --
This helps people (or automated tools) determine how far to backport
I beg pardon if I'm rehearsing an old debate, but it seems to me it
would be better and worthwhile to bring more of git to bear by adding
`reference` links as follows from considering this proposed sequence:
# ...G---B---...history-with-bug-at-B
Gprime=`git commit-tree --reference G`
Bprime=`git commit-tree --reference B -p $Gprime`
# ...G---B---... history-with-bug-at-B
# : : # -- `:`'s are `reference` links
# G'--B'$Bprime is a mergeable cherry-pick for B
`reference` links have no enforced semantics. Teach all current logic to
ignore them (fetch doesn't fetch through them, fsck doesn't care, etc.).
Elaborating some of the good parts:
* If the author and committer data are left untouched when
`commit-tree`'s tree and message arguments are defaulted, as above, to
the referenced commit's tree and message, the resulting commit is unique.
* Bullet-proof cherry-pick creation becomes easy and idempotent:
git-make-cherry-pick() {
local picked=$1
set -- `git rev-list --parents $picked^!`
shift
local parents
local parent
local p2
for parent; do
p2=$p2 -p `git commit-tree --reference $parent`
done
git commit-tree --reference $picked $parents`
}
* Which makes the created commit id a fully-implemented _change-id_ for
the referenced commit:
git merge $(git-make-cherry-pick $B)
can be done from anywhere, merge won't have to rely on patch-id's
to detect cherry-picks done this way.
* A bugged commit gets fixed by fixing its reference commit and merging
normally, worry-free:
...G---B ... -F Merge fix X for a bug in B
: : /
G'--B'---X X's commit message is the `Fixes:` equivalent
Bugfix commit X can be safely merged anywhere. Worst case, `git
merge -s ours --no-commit X` and do whatever you would have done otherwise.
`merge` might usefully be updated to warn about merging from a commit
with only a reference parent, I think merging from `G'` would probably
be a mistake.
---
So, this is as far as I've gotten with this, is there reason to think it
should or shouldn't be pursued?
--
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] commit: Add -f, --fixes commit option to add Fixes: line
There are unbound number of kinds of trailers people would want to add, depending on their projects' needs. We should not have to add a specific support for a tailer like this one, before thinking through to see if we can add generic support for adding arbitrary trailers to avoid code and interface bloat. Think of the existing --signoff as a historical mistake. Such a generic adding arbitrary trailers support, when done properly, should be able to express what --signoff does, and we should be able to redo --signoff as a special case of that generic adding arbitrary trailers support, and at that point, Fixes: trailer the kernel project wants to use should fall out as a natural consequence. -- 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] commit: Add -f, --fixes commit option to add Fixes: line
On Sun, Oct 27, 2013 at 8:04 PM, Christian Couder christian.cou...@gmail.com wrote: On Sun, Oct 27, 2013 at 2:30 PM, Johan Herland jo...@herland.net wrote: On Sun, Oct 27, 2013 at 1:30 PM, Christian Couder christian.cou...@gmail.com wrote: Your suggestion is very good, and it is not incompatible with command line options. So both could be implemented and even work together. For example if -f ack:Peff was passed to the command line, git commit could lookup in the commit message template and see if there is one RFC822-style header that starts with or contains ack (discarding case) and it could look in some previous commits if there is an author whose name contains Peff (discarding case) ...may be cheaper to (first) look at the .mailmap? Ok. I haven't really had a look at how it could best be done. and if it is the case it could append the following to the bottom of the commit message: Fixes: Reported-by: Suggested-by: Improved-by: Acked-by: Jeff King p...@peff.net Reviewed-by: Tested-by: Signed-off-by: Myself mys...@example.com (I suppose that the sob is automatically added.) It would work also with -f fix:security-bug and would put something like what you suggested: Fixes: 1234beef56 (Commit message summmary) Even better: Imagine -f (or whatever is decided) as a general mechanism for forwarding parameters to the prepare-commit-msg hook. When you run git commit -f ack:Peff -f fix:security-bug, the -f arguments will be forwarded to prepare-commit-msg (as additional command-line args, or on stdin), and then the prepare-commit-msg hook can do whatever it wants with them (e.g. the things you describe above). If git commit processes these arguments and puts the result in the commit message file that is passed to the prepare-commit-msg hook, then this hook can still get them from the file and process them however it wants. And in most cases the processing could be the same as what is done by the commit-msg hook when the user changes the Fixes: xxx and Stuffed-by: yyy lines in the editor. So it would probably be easier for people customizing the prepare-commit-msg and commit-msg if git commit processes the arguments instead of just passing them to the prepare-commit-msg hook. And it will be better for people who don't set up any *commit-msg hook. Even if there is no commit template, -f Acked-by:Peff and -f Fixes:security-bug could still work. I suspect most users don't setup any hook or commit template. Hmm. I'm not sure what you argue about which part of the system should perform which function. Let's examine the above options in more detail. Roughly, the flow of events look like this git commit -f ack:Peff -f fix:security-bug | v builtin/commit.c (i.e. inside git commit) | v prepare-commit-msg hook | v commit message template: Fixes: security-bug Acked-by: Peff | v user edits commit message (may or may not change Fixes/Acked-by lines) | v commit-msg hook | v commit message: Fixes: 1234beef56 (Commit message summmary) Acked-by: Jeff King p...@peff.net (The above is even a bit simplified, but I believe it's sufficient for the current discussion.) So, there are several expansions happening between the initial git commit and the final commit message. They are: 1. fix - Fixes: 2. security-bug - 1234beef56 (Commit message summmary) 3. ack - Acked-by: 4. Peff - Jeff King p...@peff.net First, I think we both agree that expansions #2 and #4 MUST be done by the commit-msg hook. The reason for this is two-fold: (a) the expansion must be done (at least) after the user has edited the commit message (since the values entered by the user might require the same expansion), and (b) how (and whether) to perform the expansion is a project-specific policy question, and not something that Git can dictate. Obviously, common functionality can be made available in the default hook shipped by Git, but it's up to each project to enable and/or customize this. Second, there is #1 and #3, the expansion of ack - Acked-by: and fix - Fixes:. Is this expansion performed by the prepare-commit-msg hook, or directly inside builtin/commit.c? If you are arguing for the latter (and I'm not sure that you are), we would need to add a dictionary to git commit that maps shorthand field names (ack) to the RFC822 -style equivalent (Acked-by: ). I would instead argue for the former, i.e. simply forwarding ack and fix as-is to the prepare-commit-msg hook, and let it deal with the appropriate expansion. The main reason for this is that if a project wants to add another shorthand expansion (e.g. bug - Related-Bugzilla-Id: ), they can do so without hacking builtin/commit.c. Certainly, we could ship a default prepare-commit-msg hook that knows how to expand the usual suspects (like ack and fix), but hardcoding this inside git commit is not optimal, IMHO. The reason I like this, is that we can now support
Re: git-completion.bash error
[re-adding git mailing list] Please do use Reply All and respond inline (as below) rather than top-posting. On Sun, Oct 27, 2013 at 8:47 PM, Gabriel gabriel...@gmail.com wrote: I have a possible broken shell pipe. grep/egrep works fine when I pass a pattern and a file or folder, but it breaks after pipe. The content of the line that breaks on my machine is: git help -a|egrep '^ [a-zA-Z0-9]' Are you using a broken egrep? On Mavericks, mine shows: % type egrep egrep is /usr/bin/egrep % egrep --version egrep (BSD grep) 2.5.1-FreeBSD This commands breaks outputting the regexp not being a valid file or directory. I'll keep investigating but I've never seen a broken shell pipe function =( Thanks On Sun, Oct 27, 2013 at 8:34 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Sun, Oct 27, 2013 at 4:59 PM, Gabriel gabriel...@gmail.com wrote: I've just made a clean install of OS X Mavericks and installed Git via Homebrew, which obviously includes the contrib files. I sourced the git-completion.bash into my profile and I get stuck on a error every time I try to use the autocomplete. The error outputted is: egrep: ^ [a-zA-Z0-9]: No such file or directory I believe this is related to this line (this commands outputs the same error when entered directly): https://github.com/git/git/blob/master/contrib/completion/git-completion.bash#L614 Unable to reproduce on Mavericks either via the completion script or manual entry. I don't know if this issue is directly related to OS X Mavericks or anything else. -- 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
