Re: [RFC/PATCH] Port branch.c to use ref-filter APIs
On Tue, Jul 28, 2015 at 11:23 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: On Tue, Jul 28, 2015 at 7:05 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Ideally, you could also have a modifier atom %(alignleft) [...] This could work for refname, as each ref_array_item holds the refname, But other atoms are only filled in after a call to verify_ref_format(). If you swap the order and call verify_ref_format() after filter_refs(), then you could detect %(alignleft) and iterate over the list as needed in verify_ref_format(). (You would actually force swapping the order and you would need to adapt other callers in for-each-ref and tag) But I don't see this as priority for now, so will mark it off for later. Absolutely, that's what I meant by Ideally. I'm just thinking aloud. Maybe after GSoC ;-) -- Regards, Karthik Nayak -- 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: [RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs
Re-sending this as it wasn't sent as plain text and failed.
On Tue, Jul 28, 2015 at 7:47 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Karthik Nayak karthik@gmail.com writes:
@@ -458,7 +345,7 @@ static void add_verbose_info(struct strbuf *out, struct
ref_array_item *item,
}
if (item-kind == REF_LOCAL_BRANCH)
- fill_tracking_info(stat, item-refname, filter-verbose 1);
+ fill_tracking_info(stat, refname, filter-verbose 1);
Why can't you continue using item-refname?
(It's a real question)
Because we call add_verbose_info() is called in print_ref_item() which calls
add_verbose_info() with refname=desc, where desc is a shortened refname.
And fill_tracking_info expects a shortened refname. hence we give it refname
which is nothing but desc.
@@ -635,14 +495,21 @@ static void print_ref_list(struct ref_filter *filter)
/* Print detached heads before sorting and printing the rest */
if (filter-detached) {
print_ref_item(array.items[index - 1], maxwidth, filter,
remote_prefix);
- index -= 1;
+ array.nr--;
}
- qsort(array.items, index, sizeof(struct ref_array_item *), ref_cmp);
+ if (!sorting) {
+ def_sorting.next = NULL;
+ def_sorting.atom = parse_ref_filter_atom(sort_type,
+ sort_type +
strlen(sort_type));
+ sorting = def_sorting;
+ }
+ ref_array_sort(sorting, array);
Does this belong to print_ref_list()? Is it not possible to extract it
to get a code closer to the simple:
filter_refs(...);
ref_array_sort(...);
print_ref_list(...);
?
We have a ref_defaulting_sorting but that defaults to sorting by 'refname'
but what we want in branch.c is to sort by 'type' rather. Hence we
need to have this small segment of code. About its placement, IDK if
print_ref_list() is the right place. But didn't find a better place.
- for (i = 0; i index; i++)
+ for (i = 0; i array.nr; i++)
print_ref_item(array.items[i], maxwidth, filter,
remote_prefix);
Now that we have show_ref_array_item, it may make sense to rename
print_ref_item to something that make the difference between these
functions more explicit. Well, ideally, you'd get rid of it an actually
use show_ref_array_item, but if you are to keep it, maybe
print_ref_item_default_branch_format (or something shorter)?
I guess it'll be converted to a helper for show_ref_array_item()
eventually so keeping that in mind maybe rename it to
get_format_and_print_ref().
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -49,7 +49,6 @@ struct ref_sorting {
struct ref_array_item {
unsigned char objectname[20];
int flag, kind;
- int ignore : 1;
You should explain why you needed it and why you don't need it anymore
(I guess, because it was used to implement --merge and you now get it
from ref-filter).
Yeah, I'll add that into the commit message.
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -38,7 +38,7 @@ test_expect_success 'fast-import: fail on invalid branch
name bad[branch]name'
test_must_fail git fast-import input
'
-test_expect_success 'git branch shows badly named ref' '
+test_expect_failure 'git branch does not show badly named ref' '
I'm not sure what's the convention, but I think the test description
should give the expected behavior even with test_expect_failure.
And please help the reviewers by saying what's the status wrt this test
(any plan on how to fix it?).
Well okay will rename the test description.
Since we use filter_refs() there's no real fix for this, ref_filter_handler()
skips over such refs. I'll mention something on these lines in the
commit message.
--
Regards,
Karthik Nayak
--
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: [RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs
Karthik Nayak karthik@gmail.com writes: On Tue, Jul 28, 2015 at 7:47 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: I'm not sure what's the convention, but I think the test description should give the expected behavior even with test_expect_failure. And please help the reviewers by saying what's the status wrt this test (any plan on how to fix it?). On the other hand I wonder if the test is even needed as, we don't really need it Cause we remove that ability of branch.c by using filter_refs(). Please read d0f810f (refs.c: allow listing and deleting badly named refs, 2014-09-03). I think the reasoning makes sense, and we should keep this ability. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: [RFC/PATCH 01/11] ref-filter: add %(objectname:size=X) option
On Tue, Jul 28, 2015 at 9:49 PM, Junio C Hamano gits...@pobox.com wrote: Karthik Nayak karthik@gmail.com writes: From: Karthik Nayak karthik@gmail.com Add support for %(objectname:size=X) where X is a number. This will print the first X characters of an objectname. The minimum value for X is 5. Hence any value lesser than 5 will default to 5 characters. Where does this hardcoded 5 come from? I'd agree that we would want some minimum for sanity (not safety), but I do not think we want random callers of find-unique-abbrev arbitrarily imposing their own minimum, making different codepaths behave inconsistently without a good reason. It seems that the minimum we use for sanity at the core level is MINIMUM_ABBREV. Is there a reason why that value is inappropriate for this codepath? I don't quite remember, This is definitely wrong. Thanks Will use MINIMUM_ABBREV. -- Regards, Karthik Nayak -- 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] ref-filter: fix indentation
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
This is meant to be applied on top of kn/for-each-ref.
ref-filter.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 43502a4..3fbbbeb 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -868,8 +868,8 @@ static int ref_filter_handler(const char *refname, const
struct object_id *oid,
struct ref_array_item *ref;
if (flag REF_BAD_NAME) {
- warning(ignoring ref with broken name %s, refname);
- return 0;
+ warning(ignoring ref with broken name %s, refname);
+ return 0;
}
if (*filter-name_patterns
!match_name_as_path(filter-name_patterns, refname))
--
2.5.0.rc0.7.ge1edd74.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: Log messages beginning # and git rebase -i
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Duy Nguyen pclo...@gmail.com writes: On Wed, Jul 29, 2015 at 12:48 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: If the user wants whatever she types in the resulting commit literally, there is the --cleanup=choice option, no? $ GIT_EDITOR=touch git commit --cleanup=verbatim [detached HEAD 1b136a7] # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # HEAD detached from 5e70007 # Changes to be committed: # modified: foo.txt # # Changes not staged for commit : # modified: foo.txt # # Untracked files: # last-synchro.txt # 1 file changed, 1 insertion(+), 1 deletion(-) You really don't want that in day-to-day use. I do not quite follow this example. The user said I'll be responsible for cleaning up by giving the option. It is up to the user to use an editor that is something a bit more intelligent than touch to remove the instructional comments meant for humans after reading them. Yes, --cleanup=verbatim does what it says it does. Now, my claim is that it does not answer the use-case I want an easy way to talk about # in a commit message. First, you have to specify --cleanup=verbatim _before_ typing the message, hence before knowing that you may need a #. Then, as you say, it is up to the user to remove things that Git has added. Why would we ask the user to do this when we have a way to have the tool do it? 2) Modify Git to add scissors by default, and use --cleanup=scissors by default. I just did $ git commit --amend --cleanup=scissors (with and without --amend) and it seems to do exactly that ;-). Ah, I did my test in the same repo I messed-up with --cleanup=verbatim. It's better than I thought then. So a viable alternative to the backslas-escaping would be to change commit.cleanup to scissors by default. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 v6 01/10] ref-filter: introduce 'ref_formatting_state'
On Tue, Jul 28, 2015 at 12:56 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Karthik Nayak karthik@gmail.com writes:
-static void print_value(struct atom_value *v, int quote_style)
+static void apply_formatting_state(struct ref_formatting_state *state,
+struct atom_value *v, struct strbuf *value)
{
- struct strbuf sb = STRBUF_INIT;
- switch (quote_style) {
+ /* Eventually we'll formatt based on the ref_formatting_state */
s/formatt/format/
Also, we usually use a single space after /*.
(Neither is very important since it disapears in the next commit)
+ /*
+ * Some (pesudo) atoms have no immediate side effect, but only
s/pesudo/pseudo/. But if you are to rename these atoms to modifier
atoms, you should get rid of this pseudo here.
Ah, I hate making grammatical errors, Even though I check it always gets away.
Anyways waiting for Junio and others to reply on this version. Could do a resend
for this series if needed.
--
Regards,
Karthik Nayak
--
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 v6 01/10] ref-filter: introduce 'ref_formatting_state'
Karthik Nayak karthik@gmail.com writes: Ah, I hate making grammatical errors, Even though I check it always gets away. Anyways waiting for Junio and others to reply on this version. Could do a resend for this series if needed. If you took all my remarks into account, I think it's worth a resend as it should make it easier for new reviewers. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 v5 07/11] ref-filter: add option to match literal pattern
On Wed, Jul 29, 2015 at 3:19 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Mon, Jul 27, 2015 at 3:27 AM, Karthik Nayak karthik@gmail.com wrote:
From: Karthik Nayak karthik@gmail.com
Since 'ref-filter' only has an option to match path names add an
option for plain fnmatch pattern-matching.
This is to support the pattern matching options which are used in `git
tag -l` and `git branch -l` where we can match patterns like `git tag
-l foo*` which would match all tags which has a foo* pattern.
Signed-off-by: Karthik Nayak karthik@gmail.com
---
diff --git a/ref-filter.c b/ref-filter.c
index 26eb26c..597b189 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -946,6 +946,32 @@ static int commit_contains(struct ref_filter *filter,
struct commit *commit)
/*
* Return 1 if the refname matches one of the patterns, otherwise 0.
+ * A pattern can be a literal prefix (e.g. a refname refs/heads/master
+ * matches a pattern refs/heads/mas) or a wildcard (e.g. the same ref
+ * matches refs/heads/mas*, too).
+ */
+static int match_pattern(const char **patterns, const char *refname)
+{
+ /*
+* When no '--format' option is given we need to skip the prefix
+* for matching refs of tags and branches.
+*/
+ if (skip_prefix(refname, refs/tags/, refname))
+ ;
+ else if (skip_prefix(refname, refs/heads/, refname))
+ ;
+ else if (skip_prefix(refname, refs/remotes/, refname))
+ ;
Or, more concisely:
skip_prefix(refname, refs/tags/, refname) ||
skip_prefix(refname, refs/heads/, refname) ||
skip_prefix(refname, refs/remotes/, refname);
since || short-circuits. No need for the 'if' or cascading 'else if's.
+ for (; *patterns; patterns++) {
+ if (!wildmatch(*patterns, refname, 0, NULL))
+ return 1;
+ }
+ return 0;
+}
Looks better thanks :)
--
Regards,
Karthik Nayak
--
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 v5 03/11] ref-filter: add option to pad atoms to the right
On Wed, Jul 29, 2015 at 3:11 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Mon, Jul 27, 2015 at 3:27 AM, Karthik Nayak karthik@gmail.com wrote: Add a new atom padright and support %(padright:X) where X is a number. This will align the succeeding atom value to the left followed by spaces for a total length of X characters. If X is less than the item size, the entire atom value is printed. Signed-off-by: Karthik Nayak karthik@gmail.com --- diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index e49d578..45dd7f8 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -127,6 +127,12 @@ color:: Change output color. Followed by `:colorname`, where names are described in `color.branch.*`. +padright:: + Pad succeeding atom to the right. Followed by `:value`, + where `value` states the total length of atom including the + padding. If the `value` is greater than the atom length, then + no padding is performed. Isn't this backward? Don't you mean ... If the atom length is greater than `value`, then no padding is performed. ? Yes, silly mistake. -- Regards, Karthik Nayak -- 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: Log messages beginning # and git rebase -i
Matthieu Moy matthieu@grenoble-inp.fr writes: Then, as you say, it is up to the user to remove things that Git has added. Why would we ask the user to do this when we have a way to have the tool do it? The timeline of development, perhaps? I thought cleanup=scissors was a fairly recent invention that hasn't been used widely yet. -- 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: [RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs
Karthik Nayak karthik@gmail.com writes:
On Tue, Jul 28, 2015 at 7:47 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
- qsort(array.items, index, sizeof(struct ref_array_item *), ref_cmp);
+ if (!sorting) {
+ def_sorting.next = NULL;
+ def_sorting.atom = parse_ref_filter_atom(sort_type,
+ sort_type +
strlen(sort_type));
+ sorting = def_sorting;
+ }
+ ref_array_sort(sorting, array);
Does this belong to print_ref_list()? Is it not possible to extract it
to get a code closer to the simple:
filter_refs(...);
ref_array_sort(...);
print_ref_list(...);
?
We have a ref_defaulting_sorting but that defaults to sorting by 'refname'
but what we want in branch.c is to sort by 'type' rather. Hence we
need to have this small segment of code. About its placement, IDK if
print_ref_list() is the right place. But didn't find a better place.
Hmm, actually I think I misread the code: print_ref_list() does follow
the pattern filter_refs - sort - print.
Perhaps the function name is misleading, and you could make it clearer
that it computes the list and displays it. I don't know.
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -38,7 +38,7 @@ test_expect_success 'fast-import: fail on invalid branch
name bad[branch]name'
test_must_fail git fast-import input
'
-test_expect_success 'git branch shows badly named ref' '
+test_expect_failure 'git branch does not show badly named ref' '
I'm not sure what's the convention, but I think the test description
should give the expected behavior even with test_expect_failure.
And please help the reviewers by saying what's the status wrt this test
(any plan on how to fix it?).
Well okay will rename the test description.
Since we use filter_refs() there's no real fix for this, ref_filter_handler()
skips over such refs.
Either there's a good rationale for ignoring these refs, and you should
change the test keeping it test_expect_success (i.e. the test
observed corresponds to the expected behavior), or you should
eventually modify filter_refs() to allow not dropping them.
This
-test_expect_success '...'
+test_expect_failure '...'
explicitly says I have a known regression in this patch.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: Log messages beginning # and git rebase -i
Matthieu Moy matthieu@grenoble-inp.fr writes: Duy Nguyen pclo...@gmail.com writes: On Wed, Jul 29, 2015 at 12:48 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: If the user wants whatever she types in the resulting commit literally, there is the --cleanup=choice option, no? $ GIT_EDITOR=touch git commit --cleanup=verbatim [detached HEAD 1b136a7] # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # HEAD detached from 5e70007 # Changes to be committed: # modified: foo.txt # # Changes not staged for commit : # modified: foo.txt # # Untracked files: # last-synchro.txt # 1 file changed, 1 insertion(+), 1 deletion(-) You really don't want that in day-to-day use. I do not quite follow this example. The user said I'll be responsible for cleaning up by giving the option. It is up to the user to use an editor that is something a bit more intelligent than touch to remove the instructional comments meant for humans after reading them. 2) Modify Git to add scissors by default, and use --cleanup=scissors by default. I just did $ git commit --amend --cleanup=scissors (with and without --amend) and it seems to do exactly that ;-). -- 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 2/6] notes: replace pseudorefs with real refs
Junio C Hamano gits...@pobox.com writes: If we do not change anything (not even applying the [v3 2/6] patch we are discussing),... This one and... If there are reasons/limitations that make simultaneous notes merge of different notes in different $GIT_DIRs impossible, then I agree we shouldn't bother with [v3 2/6] patch. We should just ... this one and ... declare do not do it, it does not (yet) work. But if there isn't, [v3 2/6] is the absolute minimum thing we could ... this one. All of these reference to [v3 2/6] are wrong. The patch I meant was [PATCH] notes: handle multiple worktrees, http://thread.gmane.org/gmane.comp.version-control.git/274803/focus=274852 Sorry for the confusion. do to make notes merge usable by making sure that the user does not attempt merging the same refs/notes/commits in two different places. -- 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 0/6] clone: fix repo name when cloning a server's root
As proposed the previous patch has been changed such that we now utilize parse_connect_url(), which splits a connect URL by host and path components. If the path component is empty we now try to fall back to the host component, stripping it of its port and authentication information. The first two patches fix the t1509-root-worktree tests. They currently were not working due to broken -chains and are required to test cloning from /. The next two patches expose parse_connect_url(), which is then used in the fifth patch. The last patch provides tests for the behavior. To keep it simple we just use file:// URIs, which have the option of including a host in there, e.g. file://127.0.0.1/foobar. Patrick Steinhardt (6): tests: fix broken chains in t1509-root-worktree tests: fix cleanup after tests in t1509-root-worktree connect: expose parse_connect_url() connect: move error check to caller of parse_connect_url clone: fix hostname parsing when guessing dir clone: add tests for cloning / without workdir name builtin/clone.c | 73 +++- connect.c| 19 +++-- connect.h| 13 + t/t1509-root-worktree.sh | 43 +--- 4 files changed, 109 insertions(+), 39 deletions(-) -- 2.5.0 -- 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/6] tests: fix broken chains in t1509-root-worktree
Signed-off-by: Patrick Steinhardt p...@pks.im --- t/t1509-root-worktree.sh | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t1509-root-worktree.sh b/t/t1509-root-worktree.sh index b6977d4..0c80129 100755 --- a/t/t1509-root-worktree.sh +++ b/t/t1509-root-worktree.sh @@ -125,7 +125,7 @@ fi ONE_SHA1=d00491fd7e5bb6fa28c517a0bb32b8b506539d4d test_expect_success 'setup' ' - rm -rf /foo + rm -rf /foo mkdir /foo mkdir /foo/bar echo 1 /foo/foome @@ -218,7 +218,7 @@ unset GIT_WORK_TREE test_expect_success 'go to /' 'cd /' test_expect_success 'setup' ' - rm -rf /.git + rm -rf /.git echo Initialized empty Git repository in /.git/ expected git init result test_cmp expected result @@ -241,8 +241,8 @@ say auto bare gitdir # DESTROY! test_expect_success 'setup' ' - rm -rf /refs /objects /info /hooks - rm /* + rm -rf /refs /objects /info /hooks + rm /* cd / echo Initialized empty Git repository in / expected git init --bare result -- 2.5.0 -- 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: [RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs
On Tue, Jul 28, 2015 at 7:47 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: I'm not sure what's the convention, but I think the test description should give the expected behavior even with test_expect_failure. And please help the reviewers by saying what's the status wrt this test (any plan on how to fix it?). On the other hand I wonder if the test is even needed as, we don't really need it Cause we remove that ability of branch.c by using filter_refs(). -- Regards, Karthik Nayak -- 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: [RFC/PATCH 11/11] branch: add '--points-at' option
On Tue, Jul 28, 2015 at 1:16 PM, Jacob Keller jacob.kel...@gmail.com wrote:
On Tue, Jul 28, 2015 at 12:11 AM, Karthik Nayak karthik@gmail.com wrote:
Add the '--points-at' option provided by 'ref-filter'. The option lets
the user to list only branches which points at the given object.
Add documentation and tests for the same.
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
Documentation/git-branch.txt | 6 +-
builtin/branch.c | 7 ++-
t/t3203-branch-output.sh | 9 +
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 897cd81..efa23a5 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -11,7 +11,8 @@ SYNOPSIS
'git branch' [--color[=when] | --no-color] [-r | -a]
[--list] [-v [--abbrev=length | --no-abbrev]]
[--column[=options] | --no-column]
- [(--merged | --no-merged | --contains) [commit]] [--sort=key]
[pattern...]
+ [(--merged | --no-merged | --contains) [commit]] [--sort=key]
+ [--points-at object] [pattern...]
'git branch' [--set-upstream | --track | --no-track] [-l] [-f] branchname
[start-point]
'git branch' (--set-upstream-to=upstream | -u upstream) [branchname]
'git branch' --unset-upstream [branchname]
@@ -237,6 +238,9 @@ start-point is either a local or remote-tracking branch.
for-each-ref`. Sort order defaults to sorting based on branch
type.
+--points-at object::
+ Only list tags of the given object.
+
s/tags/branches/ ?? Since this is for the branch version, I think this
is just a copy-paste oversight.
Examples
diff --git a/builtin/branch.c b/builtin/branch.c
index 75d8bfd..d25f43b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -26,6 +26,7 @@ static const char * const builtin_branch_usage[] = {
N_(git branch [options] [-l] [-f] branch-name [start-point]),
N_(git branch [options] [-r] (-d | -D) branch-name...),
N_(git branch [options] (-m | -M) [old-branch] new-branch),
+ N_(git branch [options] [-r | -a] [--points-at]),
NULL
};
@@ -647,6 +648,10 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
OPT_COLUMN(0, column, colopts, N_(list branches in
columns)),
OPT_CALLBACK(0 , sort, sorting_tail, N_(key),
N_(field name to sort on),
parse_opt_ref_sorting),
+ {
+ OPTION_CALLBACK, 0, points-at, filter.points_at,
N_(object),
+ N_(print only tags of the object), 0,
parse_opt_object_name
+ },
Same as above. s/tags/branches/
OPT_END(),
};
@@ -675,7 +680,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
if (!delete !rename !edit_description !new_upstream
!unset_upstream argc == 0)
list = 1;
- if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE)
+ if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE ||
filter.points_at.nr)
list = 1;
if (!!delete + !!rename + !!new_upstream +
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 38c68bd..1deb7cb 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -154,4 +154,13 @@ EOF
test_i18ncmp expect actual
'
+test_expect_success 'git branch --points-at option' '
+ cat expect EOF
+ master
+ branch-one
+EOF
+ git branch --points-at=branch-one actual
+ test_cmp expect actual
+'
+
test_done
--
2.4.6
--
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
Copy paste, errors, thanks for pointing out.
--
Regards,
Karthik Nayak
--
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] ref-filter: fix indentation
On Wed, Jul 29, 2015 at 9:22 PM, Matthieu Moy matthieu@imag.fr wrote:
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
This is meant to be applied on top of kn/for-each-ref.
ref-filter.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 43502a4..3fbbbeb 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -868,8 +868,8 @@ static int ref_filter_handler(const char *refname, const
struct object_id *oid,
struct ref_array_item *ref;
if (flag REF_BAD_NAME) {
- warning(ignoring ref with broken name %s, refname);
- return 0;
+ warning(ignoring ref with broken name %s, refname);
+ return 0;
}
if (*filter-name_patterns
!match_name_as_path(filter-name_patterns, refname))
--
2.5.0.rc0.7.ge1edd74.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
Thanks for this :)
--
Regards,
Karthik Nayak
--
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 2/6] notes: replace pseudorefs with real refs
Johan Herland jo...@herland.net writes: On Wed, Jul 29, 2015 at 7:01 AM, Junio C Hamano gits...@pobox.com wrote: Johan Herland jo...@herland.net writes: I believe it is a bad compromise. It complicates the code, and it provides a concurrent notes merges that is unnecessarily tied to (and dependent on) worktrees. For example, if I wanted to perform N concurrent notes merges in a project that happens to have a huge worktree, I would now have to create N copies of the huge worktree... Who said worktree has to be populated? You should be able to have an absolutely empty checkout just like clone --no-checkout. IMHO that's an insane workaround that only serves to highlight the conceptual problems of binding notes merges (as they are implemented today) to worktrees. Actually, the name linked worktree is probably a misnomer. There is nothing fundamental in the mechanism or in the concept that says that these multiple $GIT_DIR's must not be a bare one. The main thing the separation between $GIT_DIR and $GIT_COMMON_DIR affords you is that you can have some things shared across them (e.g. refs/*, objects) while making others per $GIT_DIR (e.g. HEAD, index, etc.). With that in mind, it is not an insane workaround but a very natural mechanism suited exactly for what you want to do: using a feature (e.g. notes merge) that currently can have at most one instance running at a time because it stores its state inside $GIT_DIR, and you want to have N concurrent one going. You keep that state per running instance inside $GIT_DIR (i.e. not shared) and use the linked worktree mechanism to have multiple $GIT_DIR, connected to the same $GIT_COMMON_DIR. But, whatever. This is unrelated to David's current effort, and I don't want to hold that up, so please move along, nothing to see here. I need this part from an earlier message answered to unblock David's topic: Now we are getting somewhere. So is there more that is needed than separating NOTES_MERGE_REF per worktree to make this work (remember, multiple notes-merge in a single worktree is a non-goal, just like multiple merge in a single worktree is not supported today and will not be)? Is there some other state that is not captured by NOTES_MERGE_REF and friends that you would end up recording a wrong merge result, if two worktrees that have NOTES_MERGE_REF pointing at a different ref in refs/notes/* try to do the notes-merge at the same time? If we do not change anything (not even applying the [v3 2/6] patch we are discussing), all these things prefixed with NOTES_ will become per $GIT_DIR with linked worktrees. NOTES_EDITMSG, NOTES_MERGE_REF, NOTES_MERGE_PARTIAL, NOTES_MERGE_WORKTREE The user could attempt to start different notes merges in her multiple $GIT_DIRs. The question is to what degree we want to support her. Is it sufficient to have these per $GIT_DIR, when the user has two $GIT_DIRs connected to the same repository and wants to do two notes merge acting on different ref in refs/notes/*? Or are there some other states in the shared part kept, which would be stomped on by simultaneously running notes merge instances in different $GIT_DIRs, that make this not to work? Any other problems in the remainder of the current implementation of notes merge? If there are reasons/limitations that make simultaneous notes merge of different notes in different $GIT_DIRs impossible, then I agree we shouldn't bother with [v3 2/6] patch. We should just declare do not do it, it does not (yet) work. But if there isn't, [v3 2/6] is the absolute minimum thing we could do to make notes merge usable by making sure that the user does not attempt merging the same refs/notes/commits in two different places. -- 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 5/6] clone: fix hostname parsing when guessing dir
We fail to guess a sensible directory name for a newly cloned
repository when the path component of the URL is empty. E.g.
cloning a repository 'ssh://user:passw...@example.com/' we create
a directory 'passw...@example.com' for the clone.
Fix this by using parse_connect_url to split host and path
components and explicitly checking whether we need to fall back
to the hostname for guessing a directory name.
Signed-off-by: Patrick Steinhardt p...@pks.im
---
builtin/clone.c | 73 +
1 file changed, 53 insertions(+), 20 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index a72ff7e..4547729 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -9,6 +9,7 @@
*/
#include builtin.h
+#include connect.h
#include lockfile.h
#include parse-options.h
#include fetch-pack.h
@@ -147,34 +148,62 @@ static char *get_repo_path(const char *repo, int
*is_bundle)
static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
{
const char *end = repo + strlen(repo), *start;
+ char *host, *path;
size_t len;
char *dir;
+ parse_connect_url(repo, host, path);
/*
-* Strip trailing spaces, slashes and /.git
+* If the path component of the URL is empty (e.g. it is
+* empty or only contains a '/') we fall back to the host
+* name.
*/
- while (repo end (is_dir_sep(end[-1]) || isspace(end[-1])))
- end--;
- if (end - repo 5 is_dir_sep(end[-5])
- !strncmp(end - 4, .git, 4)) {
- end -= 5;
- while (repo end is_dir_sep(end[-1]))
+ if (!path || !*path || !strcmp(path, /)) {
+ if (*host == '\0')
+ die(No directory name could be guessed.\n
+ Please specify a directory on the command
line);
+ /*
+* Strip authentication information if it exists.
+*/
+ start = strchr(host, '@');
+ if (start)
+ start++;
+ else
+ start = host;
+ /*
+* Strip port if it exitsts.
+*/
+ end = strchr(start, ':');
+ if (!end)
+ end = start + strlen(start);
+ len = end - start;
+ } else {
+ /*
+* Strip trailing spaces, slashes and /.git
+*/
+ while (repo end (is_dir_sep(end[-1]) || isspace(end[-1])))
end--;
- }
+ if (end - repo 5 is_dir_sep(end[-5])
+ !strncmp(end - 4, .git, 4)) {
+ end -= 5;
+ while (repo end is_dir_sep(end[-1]))
+ end--;
+ }
- /*
-* Find last component, but be prepared that repo could have
-* the form remote.example.com:foo.git, i.e. no slash
-* in the directory part.
-*/
- start = end;
- while (repo start !is_dir_sep(start[-1]) start[-1] != ':')
- start--;
+ /*
+* Find last component, but be prepared that repo could have
+* the form remote.example.com:foo.git, i.e. no slash
+* in the directory part.
+*/
+ start = end;
+ while (repo start !is_dir_sep(start[-1]) start[-1] !=
':')
+ start--;
- /*
-* Strip .{bundle,git}.
-*/
- strip_suffix(start, is_bundle ? .bundle : .git , len);
+ /*
+* Strip .{bundle,git}.
+*/
+ strip_suffix(start, is_bundle ? .bundle : .git , len);
+ }
if (is_bare)
dir = xstrfmt(%.*s.git, (int)len, start);
@@ -203,6 +232,10 @@ static char *guess_dir_name(const char *repo, int
is_bundle, int is_bare)
if (out dir prev_space)
out[-1] = '\0';
}
+
+ free(host);
+ free(path);
+
return dir;
}
--
2.5.0
--
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 6/6] clone: add tests for cloning with empty path
Test behavior of `git clone` when working with an empty path
component. This may be the case when cloning a file system's root
directory or from a remote server's root.
Signed-off-by: Patrick Steinhardt p...@pks.im
---
t/t1509-root-worktree.sh | 39 +++
1 file changed, 39 insertions(+)
diff --git a/t/t1509-root-worktree.sh b/t/t1509-root-worktree.sh
index 553a3f6..acfa133 100755
--- a/t/t1509-root-worktree.sh
+++ b/t/t1509-root-worktree.sh
@@ -237,6 +237,45 @@ test_foobar_foobar
test_expect_success 'cleanup' 'rm -rf /.git'
+say clone .git at root without reponame
+
+test_expect_success 'go to /' 'cd /'
+test_expect_success 'setup' '
+ echo Initialized empty Git repository in /.git/ expected
+ git init result
+ test_cmp expected result
+'
+
+test_clone_expect_dir() {
+ URL=$1
+ DIR=$2
+ echo Cloning into '$DIR'... expected
+ echo warning: You appear to have cloned an empty repository.
expected
+ git clone $URL 2result result
+ rm -r $DIR
+ test_cmp expected result
+}
+
+test_expect_success 'go to /clones' 'mkdir /clones cd /clones'
+test_expect_success 'simple clone of /' '
+ echo fatal: No directory name could be guessed. expected
+ echo Please specify a directory on the command line expected
+ test_expect_code 128 git clone / 2result result
+ test_cmp expected result'
+
+test_expect_success 'clone with file://' '
+ test_clone_expect_dir file://127.0.0.1/ 127.0.0.1'
+test_expect_success 'clone with file://user@' '
+ test_clone_expect_dir file://user@127.0.0.1/ 127.0.0.1'
+test_expect_success 'clone with file://user:password@' '
+ test_clone_expect_dir file://user:password@127.0.0.1/ 127.0.0.1'
+test_expect_success 'clone with file://:port' '
+ test_clone_expect_dir file://127.0.0.1:/ 127.0.0.1'
+test_expect_success 'clone with file://user:password@:port' '
+ test_clone_expect_dir file://user:password@127.0.0.1:/ 127.0.0.1'
+
+test_expect_success 'cleanup' 'rm -rf /.git /clones'
+
say auto bare gitdir
# DESTROY!
--
2.5.0
--
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 2/6] tests: fix cleanup after tests in t1509-root-worktree
During cleanup we do a simple 'rm /*' to remove leftover files from previous tests. As 'rm' errors out when there is anything it cannot delete and there are directories present at '/' it will throw an error, causing the '' chain to fail. Fix this by explicitly removing the files. Signed-off-by: Patrick Steinhardt p...@pks.im --- t/t1509-root-worktree.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t1509-root-worktree.sh b/t/t1509-root-worktree.sh index 0c80129..553a3f6 100755 --- a/t/t1509-root-worktree.sh +++ b/t/t1509-root-worktree.sh @@ -242,7 +242,7 @@ say auto bare gitdir # DESTROY! test_expect_success 'setup' ' rm -rf /refs /objects /info /hooks - rm /* + rm -f /expected /ls.expected /me /result cd / echo Initialized empty Git repository in / expected git init --bare result -- 2.5.0 -- 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 3/6] connect: expose parse_connect_url()
Expose parse_connect_url which is to be used later in this patch
series.
Signed-off-by: Patrick Steinhardt p...@pks.im
---
connect.c | 13 +
connect.h | 13 +
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/connect.c b/connect.c
index c0144d8..bdbcee4 100644
--- a/connect.c
+++ b/connect.c
@@ -228,13 +228,6 @@ int server_supports(const char *feature)
return !!server_feature_value(feature, NULL);
}
-enum protocol {
- PROTO_LOCAL = 1,
- PROTO_FILE,
- PROTO_SSH,
- PROTO_GIT
-};
-
int url_is_local_not_ssh(const char *url)
{
const char *colon = strchr(url, ':');
@@ -580,11 +573,7 @@ static char *get_port(char *host)
return NULL;
}
-/*
- * Extract protocol and relevant parts from the specified connection URL.
- * The caller must free() the returned strings.
- */
-static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
+enum protocol parse_connect_url(const char *url_orig, char **ret_host,
char **ret_path)
{
char *url;
diff --git a/connect.h b/connect.h
index c41a685..245890f 100644
--- a/connect.h
+++ b/connect.h
@@ -11,4 +11,17 @@ extern int parse_feature_request(const char *features, const
char *feature);
extern const char *server_feature_value(const char *feature, int *len_ret);
extern int url_is_local_not_ssh(const char *url);
+enum protocol {
+ PROTO_LOCAL = 1,
+ PROTO_FILE,
+ PROTO_SSH,
+ PROTO_GIT
+};
+
+/*
+ * Extract protocol and relevant parts from the specified connection URL.
+ * The caller must free() the returned strings.
+ */
+extern enum protocol parse_connect_url(const char *url_orig, char **ret_host,
char **ret_path);
+
#endif
--
2.5.0
--
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 4/6] connect: move error check to caller of parse_connect_url
parse_connect_url() checks if the path component of the URL is
empty and if so causes the program to die. As the function is to
be used at other call sites which do not require this check, move
up the error checking to the existing caller.
Signed-off-by: Patrick Steinhardt p...@pks.im
---
connect.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/connect.c b/connect.c
index bdbcee4..e8b813d 100644
--- a/connect.c
+++ b/connect.c
@@ -613,9 +613,6 @@ enum protocol parse_connect_url(const char *url_orig, char
**ret_host,
else
path = strchr(end, separator);
- if (!path || !*path)
- die(No path specified. See 'man git-pull' for valid url
syntax);
-
/*
* null-terminate hostname and point path to ~ for URL's like this:
*ssh://host.xz/~user/repo
@@ -665,6 +662,9 @@ struct child_process *git_connect(int fd[2], const char
*url,
signal(SIGCHLD, SIG_DFL);
protocol = parse_connect_url(url, hostandport, path);
+ if (!path || !*path)
+ die(No path specified. See 'man git-pull' for valid url
syntax);
+
if ((flags CONNECT_DIAG_URL) (protocol != PROTO_SSH)) {
printf(Diag: url=%s\n, url ? url : NULL);
printf(Diag: protocol=%s\n, prot_name(protocol));
--
2.5.0
--
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 1/5] refs: Introduce pseudoref and per-worktree ref concepts
Add glossary entries for both concepts. Pseudorefs and per-worktree refs do not yet have special handling, because the files refs backend already handles them correctly. Later, we will make the LMDB backend call out to the files backend to handle per-worktree refs. Signed-off-by: David Turner dtur...@twopensource.com --- Documentation/glossary-content.txt | 21 + 1 file changed, 21 insertions(+) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index ab18f4b..ff14079 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -411,6 +411,27 @@ exclude;; core Git. Porcelains expose more of a def_SCM,SCM interface than the def_plumbing,plumbing. +[[def_per_worktree_ref]]per-worktree ref:: + Refs that are per-def_worktree,worktree, rather than + global. This is presently only def_HEAD,HEAD, but might + later include other unusual refs. + +[[def_pseudoref]]pseudoref:: + Pseudorefs are a class of files under `$GIT_DIR` which behave + like refs for the purposes of rev-parse, but which are treated + specially by git. Psuedorefs both have names that are all-caps, + and always start with a line consisting of a + def_sha1,SHA-1 followed by whitespace. So, HEAD is not a + pseudoref, because it is sometimes a symbolic ref. They might + optionally contain some additional data. `MERGE_HEAD` and + `CHERRY_PICK_HEAD` are examples. Unlike + def_per_worktree_ref,per-worktree refs, these files cannot + be symbolic refs, and never have reflogs. They also cannot be + updated through the normal ref update machinery. Instead, + they are updated by directly writing to the files. However, + they can be read as if they were refs, so `git rev-parse + MERGE_HEAD` will work. + [[def_pull]]pull:: Pulling a def_branch,branch means to def_fetch,fetch it and def_merge,merge it. See also linkgit:git-pull[1]. -- 2.0.4.315.gad8727a-twtrsrc -- 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 0/5] pseudorefs
This version removes the old patch 2/6, which changed notes to use normal refs instead of pseudorefs. We don't actually want to forbid per-worktree notes merge; instead, we want to either ensure that they don't conflict, or use a completely different merge mechanism. So we omit this patch. In addition, this version fixes a style issue reported by Eric Sunshine. It also fixes a lock leak and removes some redundant code; Eric also suggested both of these changes. It also corrects the word rebase with bisect in a commit message; Junio noticed this one. -- 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 2/5] refs: add ref_type function
Add a function ref_type, which categorizes refs as per-worktree,
pseudoref, or normal ref.
Later, we will use this in refs.c to treat pseudorefs specially.
Alternate ref backends may use it to treat both pseudorefs and
per-worktree refs differently.
Signed-off-by: David Turner dtur...@twopensource.com
---
refs.c | 26 ++
refs.h | 8
2 files changed, 34 insertions(+)
diff --git a/refs.c b/refs.c
index 0b96ece..0f87884 100644
--- a/refs.c
+++ b/refs.c
@@ -2848,6 +2848,32 @@ static int delete_ref_loose(struct ref_lock *lock, int
flag, struct strbuf *err)
return 0;
}
+static int is_per_worktree_ref(const char *refname)
+{
+ return !strcmp(refname, HEAD);
+}
+
+static int is_pseudoref_syntax(const char *refname)
+{
+ const char *c;
+
+ for (c = refname; *c; c++) {
+ if (!isupper(*c) *c != '-' *c != '_')
+ return 0;
+ }
+
+ return 1;
+}
+
+enum ref_type ref_type(const char *refname)
+{
+ if (is_per_worktree_ref(refname))
+ return REF_TYPE_PER_WORKTREE;
+ if (is_pseudoref_syntax(refname))
+ return REF_TYPE_PSEUDOREF;
+ return REF_TYPE_NORMAL;
+}
+
int delete_ref(const char *refname, const unsigned char *old_sha1,
unsigned int flags)
{
diff --git a/refs.h b/refs.h
index e4e46c3..dca4fb5 100644
--- a/refs.h
+++ b/refs.h
@@ -445,6 +445,14 @@ extern int parse_hide_refs_config(const char *var, const
char *value, const char
extern int ref_is_hidden(const char *);
+enum ref_type {
+ REF_TYPE_PER_WORKTREE,
+ REF_TYPE_PSEUDOREF,
+ REF_TYPE_NORMAL,
+};
+
+enum ref_type ref_type(const char *refname);
+
enum expire_reflog_flags {
EXPIRE_REFLOGS_DRY_RUN = 1 0,
EXPIRE_REFLOGS_UPDATE_REF = 1 1,
--
2.0.4.315.gad8727a-twtrsrc
--
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: [RFC/PATCH 07/11] branch: move 'current' check down to the presentation layer
On Wed, Jul 29, 2015 at 3:31 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: On Tue, Jul 28, 2015 at 6:39 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: We check if given ref is the current branch in print_ref_list(). Move this check to print_ref_item() where it is checked right before printing. This means that the '*' and the different color are coded in C, hence it's not possible to mimick this using git for-each-ref --format I do not consider this as blocking, but I think the ultimate goal should be to allow this, so that all the goodies of git branch can be made available to other ref-listing commands. Not sure what you mean here. What you already know, but probably badly explained ;-). Eventually, the output of git branch should correspond to a format string (so git branch would be almost an alias for git for-each-ref refs/heads/ --format '...'). Internally, this would mean using show_ref_array_item instead of print_ref_item. This is what you managed to do for git tag. You already identified one difficulty with sha1 alignment in git branch -v. I'm pointing out another which is that displaying the * in front of the current branch is currently not possible with a format string. You would need an atom like %(displayAStarIfTheBranchIsTheCurrentOne) (for which you'd need to find a short-and-sweet name ;-) ). What I was thinking of was something like this : struct strbuf format = STRBUF_INIT; char c = ' '; if (current) c = '*'; strbuf_addf(format, %c, c, other format options...); show_ref_array_item(item, format.buf, quote_style, 0); strbuf_release(format); This would remove the need of making the printing of the * to be needed by ref-filter. As this is only needed in branch.c If going on what you're saying we could have a %(starifcurrent) atom or something, but I don't see a general use for this. -- Regards, Karthik Nayak -- 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 v6 01/10] ref-filter: introduce 'ref_formatting_state'
Eric Sunshine sunsh...@sunshineco.com writes:
@@ -1254,9 +1273,26 @@ static void emit(const char *cp, const char *ep)
+static void reset_formatting_state(struct ref_formatting_state *state)
+{
+ int quote_style = state-quote_style;
+ memset(state, 0, sizeof(*state));
+ state-quote_style = quote_style;
I wonder if this sledge-hammer approach of saving one or two values
before clearing the entire 'ref_formatting_state' and then restoring
the saved values will scale well. Would it be better for this to just
individually reset the fields which need resetting and not touch those
that don't?
Also, the fact that quote_style has to be handled specially may be an
indication that it doesn't belong in this structure grouped with the
other modifiers or that you need better classification within the
structure.
Actually, I think it is wrong to have this function in the first
place. It is a sign that the caller is doing too little before
calling this function.
If the act of printing an atom uses the formatting state that says
next one needs X, then it is responsible to clear that next one
needs X part of the state, as it is the one who consumed that
state. E.g. if it used to say next one needs to be padded to the
right before entering print_value(), then the function did that
padded output, then the next one needs to be padded to the
right should be cleared inside print_value().
--
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 v6 01/10] ref-filter: introduce 'ref_formatting_state'
Eric Sunshine sunsh...@sunshineco.com writes:
@@ -1254,9 +1273,26 @@ static void emit(const char *cp, const char *ep)
+static void reset_formatting_state(struct ref_formatting_state *state)
+{
+ int quote_style = state-quote_style;
+ memset(state, 0, sizeof(*state));
+ state-quote_style = quote_style;
I wonder if this sledge-hammer approach of saving one or two values
before clearing the entire 'ref_formatting_state' and then restoring
the saved values will scale well. Would it be better for this to just
individually reset the fields which need resetting and not touch those
that don't?
Also, the fact that quote_style has to be handled specially may be an
indication that it doesn't belong in this structure grouped with the
other modifiers or that you need better classification within the
structure.
Actually, I think it is wrong to have this function in the first
place. It is a sign that the caller is doing too little before
calling this function.
If the act of printing an atom uses the formatting state that says
next one needs X, then it is responsible to clear that next one
needs X part of the state, as it is the one who consumed that
state. E.g. if it used to say next one needs to be padded to the
right before entering print_value(), then the function did that
padded output, then the next one needs to be padded to the
right should be cleared inside print_value().
And with that arrangement, together with calling emit() with
formatting state, %(color:blue) can be handled as a normal part of
the formatting state mechanism. The pseudo/modifier atom should
update the state to Start printing in blue, and either emit() or
print_value(), whichever is called first, would notice that state,
does what was requested, and flip that bit down (because we are
already printing in blue so the next output function does not have
to do the blue thing again).
--
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] parse-options: align curly braces for all options
Signed-off-by: Stefan Beller sbel...@google.com
---
The non alignment of white space harmed my feelings for aesthetics more
than it should have.
Hmm, but this does not align curlies for OPT_DATE(),
OPT_EXPIRY_DATE(), etc...
That's true, but that was the least invasive fix to appease the aesthetics
as it was all nicely aligned in the beginning, then there was one outlier,
and it was nice again, at the end it is a mess.
Here is the another way to make it look nicer, though not minimally but rather
extensive.
parse-options.h | 79 -
1 file changed, 39 insertions(+), 40 deletions(-)
diff --git a/parse-options.h b/parse-options.h
index c71e9da..78e4462 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -111,46 +111,45 @@ struct option {
intptr_t defval;
};
-#define OPT_END() { OPTION_END }
-#define OPT_ARGUMENT(l, h) { OPTION_ARGUMENT, 0, (l), NULL, NULL, \
- (h), PARSE_OPT_NOARG}
-#define OPT_GROUP(h){ OPTION_GROUP, 0, NULL, NULL, NULL, (h) }
-#define OPT_BIT(s, l, v, h, b) { OPTION_BIT, (s), (l), (v), NULL, (h), \
- PARSE_OPT_NOARG, NULL, (b) }
-#define OPT_NEGBIT(s, l, v, h, b) { OPTION_NEGBIT, (s), (l), (v), NULL, \
- (h), PARSE_OPT_NOARG, NULL, (b) }
-#define OPT_COUNTUP(s, l, v, h) { OPTION_COUNTUP, (s), (l), (v), NULL, \
- (h), PARSE_OPT_NOARG }
-#define OPT_SET_INT(s, l, v, h, i) { OPTION_SET_INT, (s), (l), (v), NULL, \
- (h), PARSE_OPT_NOARG, NULL, (i) }
-#define OPT_BOOL(s, l, v, h)OPT_SET_INT(s, l, v, h, 1)
-#define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \
- (h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
NULL, 1}
-#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
- (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG,
NULL, (i) }
-#define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), N_(n),
(h) }
-#define OPT_STRING(s, l, v, a, h) { OPTION_STRING, (s), (l), (v), (a), (h) }
-#define OPT_STRING_LIST(s, l, v, a, h) \
- { OPTION_CALLBACK, (s), (l), (v), (a), \
- (h), 0, parse_opt_string_list }
-#define OPT_UYN(s, l, v, h) { OPTION_CALLBACK, (s), (l), (v), NULL, \
- (h), PARSE_OPT_NOARG, parse_opt_tertiary
}
-#define OPT_DATE(s, l, v, h) \
- { OPTION_CALLBACK, (s), (l), (v), N_(time),(h), 0,\
- parse_opt_approxidate_cb }
-#define OPT_EXPIRY_DATE(s, l, v, h) \
- { OPTION_CALLBACK, (s), (l), (v), N_(expiry-date),(h), 0, \
- parse_opt_expiry_date_cb }
-#define OPT_CALLBACK(s, l, v, a, h, f) \
- { OPTION_CALLBACK, (s), (l), (v), (a), (h), 0, (f) }
-#define OPT_NUMBER_CALLBACK(v, h, f) \
- { OPTION_NUMBER, 0, NULL, (v), NULL, (h), \
- PARSE_OPT_NOARG | PARSE_OPT_NONEG, (f) }
-#define OPT_FILENAME(s, l, v, h){ OPTION_FILENAME, (s), (l), (v), \
- N_(file), (h) }
-#define OPT_COLOR_FLAG(s, l, v, h) \
- { OPTION_CALLBACK, (s), (l), (v), N_(when), (h), PARSE_OPT_OPTARG, \
- parse_opt_color_flag_cb, (intptr_t)always }
+#define OPT_END() { OPTION_END }
+#define OPT_ARGUMENT(l, h) { OPTION_ARGUMENT, 0, (l), NULL, NULL, \
+ (h), PARSE_OPT_NOARG}
+#define OPT_GROUP(h) { OPTION_GROUP, 0, NULL, NULL, NULL, \
+ (h) }
+#define OPT_BIT(s, l, v, h, b) { OPTION_BIT, (s), (l), (v), NULL, \
+ (h), PARSE_OPT_NOARG, NULL, (b) }
+#define OPT_NEGBIT(s, l, v, h, b) { OPTION_NEGBIT, (s), (l), (v), NULL, \
+ (h), PARSE_OPT_NOARG, NULL, (b) }
+#define OPT_COUNTUP(s, l, v, h){ OPTION_COUNTUP, (s), (l), (v), NULL, \
+ (h), PARSE_OPT_NOARG }
+#define OPT_SET_INT(s, l, v, h, i) { OPTION_SET_INT, (s), (l), (v), NULL, \
+ (h), PARSE_OPT_NOARG, NULL, (i) }
+#define OPT_BOOL(s, l, v, h) OPT_SET_INT(s, l, v, \
+ (h), 1)
+#define OPT_HIDDEN_BOOL(s, l, v, h){ OPTION_SET_INT, (s), (l), (v), NULL, \
+ (h), PARSE_OPT_NOARG |
PARSE_OPT_HIDDEN, NULL, 1}
+#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
+ (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG,
NULL, (i) }
+#define OPT_INTEGER(s, l, v, h){ OPTION_INTEGER, (s), (l), (v),
N_(n), \
+ (h) }
+#define OPT_STRING(s, l, v, a, h) {
Re: [PATCH v6 01/10] ref-filter: introduce 'ref_formatting_state'
Eric Sunshine sunsh...@sunshineco.com writes:
@@ -1254,9 +1273,26 @@ static void emit(const char *cp, const char *ep)
+static void reset_formatting_state(struct ref_formatting_state *state)
+{
+ int quote_style = state-quote_style;
+ memset(state, 0, sizeof(*state));
+ state-quote_style = quote_style;
I wonder if this sledge-hammer approach of saving one or two values
before clearing the entire 'ref_formatting_state' and then restoring
the saved values will scale well. Would it be better for this to just
individually reset the fields which need resetting and not touch those
that don't?
I'm the one who suggested these 3 lines. I wrote them this way with the
assumption that there would only be 1 field to keep, and thet the rest
of the series was going to add more fields to reset (currently true I
think), to avoid the risk of forgetting one value to reset.
I'm fine with the other way around too.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: [RFC/PATCH 07/11] branch: move 'current' check down to the presentation layer
Karthik Nayak karthik@gmail.com writes: What I was thinking of was something like this : struct strbuf format = STRBUF_INIT; char c = ' '; if (current) c = '*'; strbuf_addf(format, %c, c, other format options...); show_ref_array_item(item, format.buf, quote_style, 0); strbuf_release(format); I think that would interact badly with verify_ref_format(). Usually, you have just one format string and call verify_ref_format() on it, not a different format string for each ref_array_item. That would probably be solvable. This would remove the need of making the printing of the * to be needed by ref-filter. As this is only needed in branch.c If going on what you're saying we could have a %(starifcurrent) atom or something, but I don't see a general use for this. To have a really customizeable format, you would want to allow e.g. git branch --format '%(starifcurrent) %(objectname) %(refname)' if the user wants to get the sha1 before the refname, and still have the star in front. It's a bit frustrating to have a hardcoded format that the user can't reproduce with the --format option, since it means you can't easily make a small variation on it. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: [RFC/PATCH 02/11] ref-filter: add 'colornext' atom
Eric Sunshine sunsh...@sunshineco.com writes:
Also, please explain here and in the commit message why this highly
specialized colorizer ('colornext'), is needed even though a more
general purpose one ('color') is already available.
It is needed in the current form to allow
%(colornext:blue)%(ifexists:[%s]) to color only the replacement of %s
and not the [].
But I now think that this would be more elegantly solved by Junio's
%(if) %(endif) idea:
%(if:atom) [ %(color:blue)%(atom)%(color:reset) ] %(endif)
(I added spaces for clarity)
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: Junio C Hamano gits...@pobox.com writes: Couldn't think of a better replacer, any suggestions would be welcome :) See below. ... One way to do all of the above is ... Note that is just one way, not the only or not necessarily the best. It certainly is not the easiest, I think. %(if:atom)...%(endif) might be easier to implement. And I find it easier to read or write too. Nested parenthesis in a format string make them really tricky. That removes the need for escaping since the content of the if/endif is a format string like the others, it can use the same escaping rules (IIRC, %% to escape a %). Yeah, that is also a good point. -- 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] git-am: flag suspiciously old or futuristic commits
On Wed, Jul 29, 2015 at 3:01 PM, Paul Gortmaker paul.gortma...@windriver.com wrote: The linux kernel repository has some commits in it with dates from the year 1970 and also 2030 (and possibly others). We probably shouldi warn people when the dates look suspect. For commits in the future, note that a committer in Australia could commit on New Years Day, and send it to a maintainer in North America and that would trip the notification on the maintainer's New Years Eve. But that is unlikely, and the note is still correct; that the commit is from a future year. For commits in the past, I chose a somewhat arbitrary 30 year limit, which will allow stuff from post 1985; the thought being that someone might want to import an old repo into git from some other SCM. We could alternatively set it to 5, which would then catch computers with a dead CMOS battery, at the risk of pestering the hypothetical museum curator of old bits. Sample output: paul@builder:~/git/linux-head$ grep Date: *patch future.patch:Date: Sat, 18 Jul 2037 21:22:19 -0400 past.patch:Date: Sat, 18 Jul 1977 21:22:19 -0400 paul@builder:~/git/linux-head$ git am future.patch note: commit is from future year 2037. Applying: arch/sh: make heartbeat driver explicitly non-modular paul@builder:~/git/linux-head$ git reset --hard HEAD~ /dev/null paul@builder:~/git/linux-head$ git am past.patch note: commit is from implausibly old year 1977. Applying: arch/sh: make heartbeat driver explicitly non-modular paul@builder:~/git/linux-head$ Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com --- git-am.sh | 15 +++ 1 file changed, 15 insertions(+) diff --git a/git-am.sh b/git-am.sh [+cc paul tan, who rewrote am in c as a GSoC project.] index 3af351ffaaf3..ff6deb8047a4 100755 --- a/git-am.sh +++ b/git-am.sh @@ -766,6 +766,21 @@ To restore the original branch and stop patching run \\$cmdline --abort\. stop_here $this fi + if test -n $GIT_AUTHOR_DATE + then + THIS_YEAR=`date +%Y` + TOO_OLD=$(expr $THIS_YEAR - 30) + TOO_NEW=$(expr $THIS_YEAR + 1) + GIT_AUTHOR_YEAR=`date -d $GIT_AUTHOR_DATE +%Y` Would it make sense to not operate on year but on unix time, so the problem you mentioned in the commit message goes away? Another thought: Having this check in am seems a bit arbitrary to me (or rather workflow adapted ;) as we could also check in commit or pull (not sure if I actually mean the fetch or merge thereof) + + if [ $GIT_AUTHOR_YEAR -le $TOO_OLD ]; then + say $(gettext note: commit is from implausibly old year $GIT_AUTHOR_YEAR.) + fi + if [ $GIT_AUTHOR_YEAR -ge $TOO_NEW ]; then + say $(gettext note: commit is from future year $GIT_AUTHOR_YEAR.) + fi + fi + export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE case $resume in -- 2.5.0 -- 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 -- 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] git-am: flag suspiciously old or futuristic commits
The linux kernel repository has some commits in it with dates from the year 1970 and also 2030 (and possibly others). We probably shouldi warn people when the dates look suspect. For commits in the future, note that a committer in Australia could commit on New Years Day, and send it to a maintainer in North America and that would trip the notification on the maintainer's New Years Eve. But that is unlikely, and the note is still correct; that the commit is from a future year. For commits in the past, I chose a somewhat arbitrary 30 year limit, which will allow stuff from post 1985; the thought being that someone might want to import an old repo into git from some other SCM. We could alternatively set it to 5, which would then catch computers with a dead CMOS battery, at the risk of pestering the hypothetical museum curator of old bits. Sample output: paul@builder:~/git/linux-head$ grep Date: *patch future.patch:Date: Sat, 18 Jul 2037 21:22:19 -0400 past.patch:Date: Sat, 18 Jul 1977 21:22:19 -0400 paul@builder:~/git/linux-head$ git am future.patch note: commit is from future year 2037. Applying: arch/sh: make heartbeat driver explicitly non-modular paul@builder:~/git/linux-head$ git reset --hard HEAD~ /dev/null paul@builder:~/git/linux-head$ git am past.patch note: commit is from implausibly old year 1977. Applying: arch/sh: make heartbeat driver explicitly non-modular paul@builder:~/git/linux-head$ Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com --- git-am.sh | 15 +++ 1 file changed, 15 insertions(+) diff --git a/git-am.sh b/git-am.sh index 3af351ffaaf3..ff6deb8047a4 100755 --- a/git-am.sh +++ b/git-am.sh @@ -766,6 +766,21 @@ To restore the original branch and stop patching run \\$cmdline --abort\. stop_here $this fi + if test -n $GIT_AUTHOR_DATE + then + THIS_YEAR=`date +%Y` + TOO_OLD=$(expr $THIS_YEAR - 30) + TOO_NEW=$(expr $THIS_YEAR + 1) + GIT_AUTHOR_YEAR=`date -d $GIT_AUTHOR_DATE +%Y` + + if [ $GIT_AUTHOR_YEAR -le $TOO_OLD ]; then + say $(gettext note: commit is from implausibly old year $GIT_AUTHOR_YEAR.) + fi + if [ $GIT_AUTHOR_YEAR -ge $TOO_NEW ]; then + say $(gettext note: commit is from future year $GIT_AUTHOR_YEAR.) + fi + fi + export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE case $resume in -- 2.5.0 -- 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: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom
Junio C Hamano gits...@pobox.com writes: Junio C Hamano gits...@pobox.com writes: Couldn't think of a better replacer, any suggestions would be welcome :) See below. ... One way to do all of the above is ... Note that is just one way, not the only or not necessarily the best. It certainly is not the easiest, I think. %(if:atom)...%(endif) might be easier to implement. And I find it easier to read or write too. Nested parenthesis in a format string make them really tricky. That removes the need for escaping since the content of the if/endif is a format string like the others, it can use the same escaping rules (IIRC, %% to escape a %). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 3/5] pseudorefs: create and use pseudoref update and delete functions
Pseudorefs should not be updated through the ref transaction
API, because alternate ref backends still need to store pseudorefs
in GIT_DIR (instead of wherever they store refs). Instead,
change update_ref and delete_ref to call pseudoref-specific
functions.
Signed-off-by: David Turner dtur...@twopensource.com
---
refs.c | 101 +++--
1 file changed, 93 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 0f87884..e6fc3fe 100644
--- a/refs.c
+++ b/refs.c
@@ -2874,12 +2874,88 @@ enum ref_type ref_type(const char *refname)
return REF_TYPE_NORMAL;
}
+static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,
+ const unsigned char *old_sha1, struct strbuf *err)
+{
+ const char *filename;
+ int fd;
+ static struct lock_file lock;
+ struct strbuf buf = STRBUF_INIT;
+ int ret = -1;
+
+ strbuf_addf(buf, %s\n, sha1_to_hex(sha1));
+
+ filename = git_path(%s, pseudoref);
+ fd = hold_lock_file_for_update(lock, filename, LOCK_DIE_ON_ERROR);
+ if (fd 0) {
+ strbuf_addf(err, Could not open '%s' for writing: %s,
+ filename, strerror(errno));
+ return -1;
+ }
+
+ if (old_sha1) {
+ unsigned char actual_old_sha1[20];
+ read_ref(pseudoref, actual_old_sha1);
+ if (hashcmp(actual_old_sha1, old_sha1)) {
+ strbuf_addf(err, Unexpected sha1 when writing %s,
pseudoref);
+ rollback_lock_file(lock);
+ goto done;
+ }
+ }
+
+ if (write_in_full(fd, buf.buf, buf.len) != buf.len) {
+ strbuf_addf(err, Could not write to '%s', filename);
+ rollback_lock_file(lock);
+ goto done;
+ }
+
+ commit_lock_file(lock);
+ ret = 0;
+done:
+ strbuf_release(buf);
+ return ret;
+}
+
+static int delete_pseudoref(const char *pseudoref, const unsigned char
*old_sha1)
+{
+ static struct lock_file lock;
+ const char *filename;
+
+ filename = git_path(%s, pseudoref);
+
+ if (old_sha1 !is_null_sha1(old_sha1)) {
+ int fd;
+ unsigned char actual_old_sha1[20];
+
+ fd = hold_lock_file_for_update(lock, filename,
+ LOCK_DIE_ON_ERROR);
+ if (fd 0)
+ die_errno(_(Could not open '%s' for writing),
filename);
+ read_ref(pseudoref, actual_old_sha1);
+ if (hashcmp(actual_old_sha1, old_sha1)) {
+ warning(Unexpected sha1 when deleting %s, pseudoref);
+ rollback_lock_file(lock);
+ return -1;
+ }
+
+ unlink(filename);
+ rollback_lock_file(lock);
+ } else {
+ unlink(filename);
+ }
+
+ return 0;
+}
+
int delete_ref(const char *refname, const unsigned char *old_sha1,
unsigned int flags)
{
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
+ if (ref_type(refname) == REF_TYPE_PSEUDOREF)
+ return delete_pseudoref(refname, old_sha1);
+
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_delete(transaction, refname, old_sha1,
@@ -3973,17 +4049,25 @@ int update_ref(const char *msg, const char *refname,
const unsigned char *new_sha1, const unsigned char *old_sha1,
unsigned int flags, enum action_on_err onerr)
{
- struct ref_transaction *t;
+ struct ref_transaction *t = NULL;
struct strbuf err = STRBUF_INIT;
+ int ret = 0;
- t = ref_transaction_begin(err);
- if (!t ||
- ref_transaction_update(t, refname, new_sha1, old_sha1,
- flags, msg, err) ||
- ref_transaction_commit(t, err)) {
+ if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
+ ret = write_pseudoref(refname, new_sha1, old_sha1, err);
+ } else {
+ t = ref_transaction_begin(err);
+ if (!t ||
+ ref_transaction_update(t, refname, new_sha1, old_sha1,
+ flags, msg, err) ||
+ ref_transaction_commit(t, err)) {
+ ret = 1;
+ ref_transaction_free(t);
+ }
+ }
+ if (ret) {
const char *str = update_ref failed for ref '%s': %s;
- ref_transaction_free(t);
switch (onerr) {
case UPDATE_REFS_MSG_ON_ERR:
error(str, refname, err.buf);
@@ -3998,7 +4082,8 @@ int update_ref(const char *msg, const char *refname,
return 1;
}
[PATCH v4 5/5] sequencer: replace write_cherry_pick_head with update_ref
Now update_ref (via write_pseudoref) does almost exactly what
write_cherry_pick_head did, so we can remove write_cherry_pick_head
and just use update_ref.
Signed-off-by: David Turner dtur...@twopensource.com
---
sequencer.c | 23 ---
1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index c4f4b7d..554a704 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -158,23 +158,6 @@ static void free_message(struct commit *commit, struct
commit_message *msg)
unuse_commit_buffer(commit, msg-message);
}
-static void write_cherry_pick_head(struct commit *commit, const char
*pseudoref)
-{
- const char *filename;
- int fd;
- struct strbuf buf = STRBUF_INIT;
-
- strbuf_addf(buf, %s\n, sha1_to_hex(commit-object.sha1));
-
- filename = git_path(%s, pseudoref);
- fd = open(filename, O_WRONLY | O_CREAT, 0666);
- if (fd 0)
- die_errno(_(Could not open '%s' for writing), filename);
- if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
- die_errno(_(Could not write to '%s'), filename);
- strbuf_release(buf);
-}
-
static void print_advice(int show_hint, struct replay_opts *opts)
{
char *msg = getenv(GIT_CHERRY_PICK_HELP);
@@ -607,9 +590,11 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
* write it at all.
*/
if (opts-action == REPLAY_PICK !opts-no_commit (res == 0 || res
== 1))
- write_cherry_pick_head(commit, CHERRY_PICK_HEAD);
+ update_ref(NULL, CHERRY_PICK_HEAD, commit-object.sha1, NULL,
+ REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
if (opts-action == REPLAY_REVERT ((opts-no_commit res == 0) ||
res == 1))
- write_cherry_pick_head(commit, REVERT_HEAD);
+ update_ref(NULL, REVERT_HEAD, commit-object.sha1, NULL,
+ REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
if (res) {
error(opts-action == REPLAY_REVERT
--
2.0.4.315.gad8727a-twtrsrc
--
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 4/5] bisect: use update_ref
Instead of manually writing a pseudoref (in one case) and shelling out
to git update-ref (in another), use the update_ref function. This
is much simpler.
Signed-off-by: David Turner dtur...@twopensource.com
---
bisect.c | 37 -
1 file changed, 8 insertions(+), 29 deletions(-)
diff --git a/bisect.c b/bisect.c
index 857cf59..33ac88d 100644
--- a/bisect.c
+++ b/bisect.c
@@ -19,7 +19,6 @@ static struct object_id *current_bad_oid;
static const char *argv_checkout[] = {checkout, -q, NULL, --, NULL};
static const char *argv_show_branch[] = {show-branch, NULL, NULL};
-static const char *argv_update_ref[] = {update-ref, --no-deref,
BISECT_HEAD, NULL, NULL};
static const char *term_bad;
static const char *term_good;
@@ -675,34 +674,16 @@ static int is_expected_rev(const struct object_id *oid)
return res;
}
-static void mark_expected_rev(char *bisect_rev_hex)
-{
- int len = strlen(bisect_rev_hex);
- const char *filename = git_path(BISECT_EXPECTED_REV);
- int fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);
-
- if (fd 0)
- die_errno(could not create file '%s', filename);
-
- bisect_rev_hex[len] = '\n';
- write_or_die(fd, bisect_rev_hex, len + 1);
- bisect_rev_hex[len] = '\0';
-
- if (close(fd) 0)
- die(closing file %s: %s, filename, strerror(errno));
-}
-
-static int bisect_checkout(char *bisect_rev_hex, int no_checkout)
+static int bisect_checkout(const unsigned char *bisect_rev, int no_checkout)
{
+ char bisect_rev_hex[GIT_SHA1_HEXSZ + 1];
- mark_expected_rev(bisect_rev_hex);
+ memcpy(bisect_rev_hex, sha1_to_hex(bisect_rev), GIT_SHA1_HEXSZ + 1);
+ update_ref(NULL, BISECT_EXPECTED_REV, bisect_rev, NULL, 0,
UPDATE_REFS_DIE_ON_ERR);
argv_checkout[2] = bisect_rev_hex;
if (no_checkout) {
- argv_update_ref[3] = bisect_rev_hex;
- if (run_command_v_opt(argv_update_ref, RUN_GIT_CMD))
- die(update-ref --no-deref HEAD failed on %s,
- bisect_rev_hex);
+ update_ref(NULL, BISECT_HEAD, bisect_rev, NULL, 0,
UPDATE_REFS_DIE_ON_ERR);
} else {
int res;
res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
@@ -804,7 +785,7 @@ static void check_merge_bases(int no_checkout)
handle_skipped_merge_base(mb);
} else {
printf(Bisecting: a merge base must be tested\n);
- exit(bisect_checkout(sha1_to_hex(mb), no_checkout));
+ exit(bisect_checkout(mb, no_checkout));
}
}
@@ -948,7 +929,6 @@ int bisect_next_all(const char *prefix, int no_checkout)
struct commit_list *tried;
int reaches = 0, all = 0, nr, steps;
const unsigned char *bisect_rev;
- char bisect_rev_hex[GIT_SHA1_HEXSZ + 1];
read_bisect_terms(term_bad, term_good);
if (read_bisect_refs())
@@ -986,11 +966,10 @@ int bisect_next_all(const char *prefix, int no_checkout)
}
bisect_rev = revs.commits-item-object.sha1;
- memcpy(bisect_rev_hex, sha1_to_hex(bisect_rev), GIT_SHA1_HEXSZ + 1);
if (!hashcmp(bisect_rev, current_bad_oid-hash)) {
exit_if_skipped_commits(tried, current_bad_oid);
- printf(%s is the first %s commit\n, bisect_rev_hex,
+ printf(%s is the first %s commit\n, sha1_to_hex(bisect_rev),
term_bad);
show_diff_tree(prefix, revs.commits-item);
/* This means the bisection process succeeded. */
@@ -1003,7 +982,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
(roughly %d step%s)\n, nr, (nr == 1 ? : s),
steps, (steps == 1 ? : s));
- return bisect_checkout(bisect_rev_hex, no_checkout);
+ return bisect_checkout(bisect_rev, no_checkout);
}
static inline int log2i(int n)
--
2.0.4.315.gad8727a-twtrsrc
--
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: Bug? git config and --unset are not inverses
Dave Borowitz dborow...@google.com writes: It looks like git config --unset may leave an orphaned section, but a subsequent set adds a new section: Old news known for at least several years. Patches welcome ;-) -- 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: [RFC/PATCH 02/11] ref-filter: add 'colornext' atom
On Tuesday, July 28, 2015, Karthik Nayak karthik@gmail.com wrote:
The 'colornext' atom allows us to color only the succeeding atom with
a particular color. This resets any previous color preferences set. It
is not compatible with `padright`. This is required for printing
formats like `git branch -vv` where the upstream is colored in blue.
Can you explain here and in the commit message why %(colornext) is not
compatible with %(padright:)? Is it an implementation limitation, or a
syntax or semantic limitation, or what? Can the implementation be
fixed to make it compatible or does it require a redesign?
Also, please explain here and in the commit message why this highly
specialized colorizer ('colornext'), is needed even though a more
general purpose one ('color') is already available.
--
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] parse-options: align curly braces for all options
Signed-off-by: Stefan Beller sbel...@google.com
---
The non alignment of white space harmed my feelings for aesthetics more
than it should have.
parse-options.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/parse-options.h b/parse-options.h
index c71e9da..08d7818 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -126,7 +126,7 @@ struct option {
#define OPT_BOOL(s, l, v, h)OPT_SET_INT(s, l, v, h, 1)
#define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
NULL, 1}
-#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
+#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG|PARSE_OPT_NONEG,
NULL, (i) }
#define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), N_(n),
(h) }
#define OPT_STRING(s, l, v, a, h) { OPTION_STRING, (s), (l), (v), (a), (h) }
--
2.5.0.rc1.391.g15b60ce
--
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 v6 05/10] ref-filter: add support to sort by version
On Tuesday, July 28, 2015, Karthik Nayak karthik@gmail.com wrote: Add support to sort by version using the v:refname and version:refname option. This is achieved by using the 'versioncmp()' function as the comparing function for qsort. This option is included to support sorting by versions in `git tag -l` which will eventaully be ported to use ref-filter APIs. Add documentation and tests for the same. Signed-off-by: Karthik Nayak karthik@gmail.com --- diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 45dd7f8..2b60aee 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -151,6 +151,9 @@ For sorting purposes, fields with numeric values sort in numeric order (`objectsize`, `authordate`, `committerdate`, `taggerdate`). All other fields are used to sort in their byte-value order. +There is also an option to sort by versions, this can be done by using +the fieldname `version:refname` or in short `v:refname`. Nit: or in short reads a bit oddly. Perhaps: ...`version:refname` or its alias `v:refname`. or ...`version:refname` or the short-form `v:refname`. (I rather prefer the alias alternative.) In any case, a field name that refers to a field inapplicable to the object referred by the ref does not cause an error. It returns an empty string instead. diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh index 19ac480..68688a9 100755 --- a/t/t6302-for-each-ref-filter.sh +++ b/t/t6302-for-each-ref-filter.sh @@ -97,4 +97,40 @@ test_expect_success 'padding to the right using `padright`' ' +test_expect_success 'version sort' ' + git for-each-ref --sort=version:refname --format=%(refname:short) refs/tags/ | grep foo actual + cat expect -\EOF + foo1.3 + foo1.6 + foo1.10 + EOF + test_cmp expect actual +' + +test_expect_success 'version sort (shortened)' ' + git for-each-ref --sort=v:refname --format=%(refname:short) refs/tags/ | grep foo actual + cat expect -\EOF + foo1.3 + foo1.6 + foo1.10 + EOF + test_cmp expect actual Nit: In the earlier review when I suggested using v:refname for one of the tests in order to exercise it (in addition to version:refname), I didn't mean that you had to add another (identical) test but rather that you could have one of the existing tests use v:refname. (Not a big deal. You can leave this as is if you like. I just wanted to clarify.) +' + +test_expect_success 'reverse version sort' ' + git for-each-ref --sort=-version:refname --format=%(refname:short) refs/tags/ | grep foo actual + cat expect -\EOF + foo1.10 + foo1.6 + foo1.3 + EOF + test_cmp expect actual +' + test_done -- 2.4.6 -- 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 -- 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 v6 0/10] port tag.c to use ref-filter APIs
Eric Sunshine sunsh...@sunshineco.com writes: Side Note: --format=%(padright:X) applies to the next available atom and not to the next span. I find this more accurate as I don't see why we'd want to pad something of known length. But its up for discussion This isn't supported by the current %(padright:) syntax, but an example would be if someone wants to pad a string composed of atoms and literal strings. For instance, the user might want to right-pad the composed string %(refattribute1) glorked %(refattribute2)”. It is an excellent example that shows why something of known length argument needs to be rethought. Currently we do not need it to reimplement the canned 'tag -l' format is an OK and sensible justification to stick to the current implementation of %(padright:N), but we'd need to think if we would want to keep this limited and strange form that applies to a single atom that comes next (ignoring any literal spans) as a private implementation detail between ref-filter and git tag. Opening it up to end-users would not mean we cannot add a correctly operating variant of pad this string to the right later, but it does mean we have to maintain %(padright) in this limited form forever. My knee-jerk reaction is that we probably should not want to expose this to the end users, and to discourage its use, perhaps name it somewhat strangely (e.g. %(x-padright:N) or something). -- 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: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom
On Tue, Jul 28, 2015 at 11:27 PM, Junio C Hamano gits...@pobox.com wrote: Karthik Nayak karthik@gmail.com writes: The 'ifexists' atom allows us to print a required format if the preceeding atom has a value. If the preceeding atom has no value then the format given is not printed. e.g. to print [refname] we can now use the format %(ifexists:[%s])%(refname). A handful of huh? on the design. - The atom says if *exists* and explanation says has a value. How are they related? Does an atom whose value is an empty string has a value? Or is ifexists meant to be used only to ignore meaningless atom, e.g. %(*objectname) applied to a ref that refers to an object that is not an annotated tag? It's meant to ignore meaningless atom. atom's whose values are empty strings are ignored. - That %s looks ugly. Are there cases where a user may want to say %(ifexists:[%i]) or something other than 's' after that per-cent? Couldn't think of a better replacer, any suggestions would be welcome :) . Is it allowed to have more than one %s there? . Is it allowed to have no %s there? 1. yes its allowed to have multiple %s 2. yes no %s is also allowed. - The syntax makes the reader wonder if [] is part of the construct, or just an example of any arbitrary string, i.e. is %(ifexists:the %s can be part of arbitrary string) valid? Its given as example, is that misleading? - If an arbitrary string is allowed, is there any quoting mechanism to allow ) to be part of that arbitrary string? Nope. Haven't done anything for that. I'll look into that :) - What, if anything, is allowed to come between %(ifexists...) and the next atom like %(refname)? For example, are these valid constructs? . %(ifexists...)%(padright:20)%(refname) Doesn't work with padright, maybe we could eventually support that. . %(ifexists...) %(refname) [%(subject)] Not sure what this is. - This syntax does not seem to allow switching on an attribute to show or not to show another, e.g. if %(*objectname) makes sense, then show '%(padright:20)%(refname:short) %(*subject)' for it. Yes this doesn't do that, I can say this is a pretty basic version, we could probably work on and implement more things? This is currently to support 'git branch -vv' where we have the upstream in square brackets. -- Regards, Karthik Nayak -- 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
Bug? git config and --unset are not inverses
It looks like git config --unset may leave an orphaned section, but a subsequent set adds a new section: $ git config --file=myconfig section.foo true $ git config --file=myconfig --unset section.foo $ cat myconfig [section] $ git config --file=myconfig section.foo true $ cat myconfig [section] [section] foo = true $ git --version git version 2.5.0.rc2.392.g76e840b -- 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: Bug? git config and --unset are not inverses
The most recent round of the discussion is here: http://thread.gmane.org/gmane.comp.version-control.git/219505/focus=219524 -- 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 v6 0/10] port tag.c to use ref-filter APIs
On Tuesday, July 28, 2015, Karthik Nayak karthik@gmail.com wrote: Changes in v6: * Change the flow of commits, introduce rest_formatting_state. * Rename ref_formatting - apply_formatting_state apply_pseudo_state - store_formatting_state * Remove the patch for making color a pseudo atom, and rename pseudo_atom to modifier_atom. * Small grammatical changes. Side Note: --format=%(padright:X) applies to the next available atom and not to the next span. I find this more accurate as I don't see why we'd want to pad something of known length. But its up for discussion This isn't supported by the current %(padright:) syntax, but an example would be if someone wants to pad a string composed of atoms and literal strings. For instance, the user might want to right-pad the composed string %(refattribute1) glorked %(refattribute2)”. One possible syntax to support this sort of thing would be %(padright:n:content). For instance, using the example above: %(padright:20:%(refattribute1) glorked %(refattribute2)) Another would be %(padright:n)content%(end). The %(end) token could work with other formatting directives, such as left padding, alignment, etc. For instance: %(padright:20)%(refattribute1) glorked %(refattribute2)%(end) In fact, this sort of thing has come up multiple times, with the most general being actual embedding of a full-blown scripting language to support pretty much any desired formatting. Other less ambitious proposals have also been suggested. This isn't a suggestion that you should implement any of these, but just a heads-up that, when it comes to formatting, it's not at all uncommon for people to come up with needs not anticipated by the design. -- 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: [RFC/PATCH 09/11] branch.c: use 'ref-filter' data structures
On Wed, Jul 29, 2015 at 3:38 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Karthik Nayak karthik@gmail.com writes:
On Tue, Jul 28, 2015 at 7:18 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Christian Couder christian.cou...@gmail.com writes:
On Tue, Jul 28, 2015 at 8:56 AM, Karthik Nayak karthik@gmail.com
wrote:
+static void ref_array_append(struct ref_array *array, const char
*refname)
+{
+ size_t len = strlen(refname);
+ struct ref_array_item *ref = xcalloc(1, sizeof(struct
ref_array_item) + len + 1);
+ memcpy(ref-refname, refname, len);
+ ref-refname[len] = '\0';
This looks very much like new_ref_array_item, except that the later also
takes an objectname parameter. I find it suspicious that you leave the
objectname field uninitialized.
Well the objectname is not required here, the idea is to retain the way
branch.c
works.
Why is this code not calling new_ref_array_item?
Because no objectname is there.
You do have the object_id in the scope of the call-site, so why not use
it?
(Well, in any case, do as you think is best, it's temporary throw-away
code, we shouldn't loose too much time polishing it)
Also the fact the new_ref_array_item() is static and not shared. Wouldn't make
sense to make it an API only for a single commit, when the code itself
is temporary.
The function disapears in the next commit, but I also think that this
function deserves to exist in ref-filter.{c,h} and remain after the end
of the series.
Why though? I don't see the need when new_ref_array_item() is present.
OK, if the function is specificly for append an item but leave the
objectname uninitialized, it's too specific to be useful somewhere
else. But then, make it more explicit in the name of the function and/or
in a comment.
Will add a comment :)
--
Regards,
Karthik Nayak
--
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 v6 01/10] ref-filter: introduce 'ref_formatting_state'
On Tuesday, July 28, 2015, Karthik Nayak karthik@gmail.com wrote:
Introduce 'ref_formatting' structure to hold values of pseudo atoms
which help only in formatting. This will eventually be used by atoms
like `color` and the `padright` atom which will be introduced in a
later patch.
Isn't this commit message outdated now that you no longer treat color
specially and since the terminology is changing from pseudo to
modifier? Also, isn't the structure now called
'ref_formatting_state' rather than 'ref_formatting'?
Signed-off-by: Karthik Nayak karthik@gmail.com
---
diff --git a/ref-filter.c b/ref-filter.c
index 7561727..a919a14 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -620,7 +622,7 @@ static void populate_value(struct ref_array_item *ref)
const char *name = used_atom[i];
struct atom_value *v = ref-value[i];
int deref = 0;
- const char *refname;
+ const char *refname = NULL;
What is this change about? It doesn't seem to be related to anything
else in the patch.
const char *formatp;
struct branch *branch = NULL;
@@ -1190,30 +1192,47 @@ void ref_array_sort(struct ref_sorting *sorting,
struct ref_array *array)
+static void print_value(struct atom_value *v, struct ref_formatting_state
*state)
+{
+ struct strbuf value = STRBUF_INIT;
+ struct strbuf formatted = STRBUF_INIT;
+
+ /*
+* Some (pesudo) atoms have no immediate side effect, but only
+* affect the next atom. Store the relevant information from
+* these atoms in the 'state' variable for use when displaying
+* the next atom.
+*/
+ apply_formatting_state(state, v, value);
The comment says that this is storing formatting state, however, the
code is actually applying the state. You could move this comment
down to show_ref_array_item() where formatting state actually gets
stored. Or you could fix it to talk about applying the state.
However, now that apply_formatting_state() has a meaningful name, you
could also drop the comment altogether since it doesn't say much
beyond what is said already by the function name.
+ switch (state-quote_style) {
case QUOTE_NONE:
- fputs(v-s, stdout);
@@ -1254,9 +1273,26 @@ static void emit(const char *cp, const char *ep)
+static void reset_formatting_state(struct ref_formatting_state *state)
+{
+ int quote_style = state-quote_style;
+ memset(state, 0, sizeof(*state));
+ state-quote_style = quote_style;
I wonder if this sledge-hammer approach of saving one or two values
before clearing the entire 'ref_formatting_state' and then restoring
the saved values will scale well. Would it be better for this to just
individually reset the fields which need resetting and not touch those
that don't?
Also, the fact that quote_style has to be handled specially may be an
indication that it doesn't belong in this structure grouped with the
other modifiers or that you need better classification within the
structure. For instance:
struct ref_formatting_state {
struct global {
int quote_style;
};
struct local {
int pad_right;
};
where 'local' state gets reset by reset_formatting_state(), and
'global' is left alone.
That's just one idea, not necessarily a proposal, but is something to
think about since the current arrangement is kind of yucky.
+}
+
void show_ref_array_item(struct ref_array_item *info, const char *format,
int quote_style)
{
const char *cp, *sp, *ep;
+ struct ref_formatting_state state;
+
+ memset(state, 0, sizeof(state));
+ state.quote_style = quote_style;
It's a little bit ugly to use memset() here when you have
reset_formatting_state() available. You could set quote_style first,
and then call reset_formatting_state() rather than memset(). Or,
perhaps, change reset_formatting_state(), as described above, to stop
using the sledge-hammer approach.
for (cp = format; *cp (sp = find_next(cp)); cp = ep + 1) {
struct atom_value *atomv;
--
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: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom
Karthik Nayak karthik@gmail.com writes: A handful of huh? on the design. - The atom says if *exists* and explanation says has a value. How are they related? Does an atom whose value is an empty string has a value? Or is ifexists meant to be used only to ignore meaningless atom, e.g. %(*objectname) applied to a ref that refers to an object that is not an annotated tag? It's meant to ignore meaningless atom. atom's whose values are empty strings are ignored. That is a self-contradicting answer. If you ask for %(*objectname) on a commit, that request truly is meaningless, as a commit is not an annotated tag that points at another object whose objectname is being asked for. But if a commit has an empty log message (you should be able to create such an object with commit-tree), then %(subject) would be an empty string. The fact that the commit happens to have an empty string as its message is far from meaningless. Either you ignore an empty string, or you ignore meaningless one. Which does ifexists mean? - That %s looks ugly. Are there cases where a user may want to say %(ifexists:[%i]) or something other than 's' after that per-cent? Couldn't think of a better replacer, any suggestions would be welcome :) See below. Its given as example, is that misleading? Othewise I wouldn't be asking. - What, if anything, is allowed to come between %(ifexists...) and the next atom like %(refname)? For example, are these valid constructs? . %(ifexists...)%(padright:20)%(refname) Doesn't work ... ... - This syntax does not seem to allow switching on an attribute to show or not to show another, e.g. if %(*objectname) makes sense, then show '%(padright:20)%(refname:short) %(*subject)' for it. Yes this doesn't do that, One way to do all of the above is to make it %(ifexists:atom:expansionString) That is, for example: %(ifexists:*objectname:tag %(color:blue)%(refname:short)%(color:reset)) would give you a string tag v1.0 with v1.0 painted in blue for refs/tags/v1.0 but nothing for refs/heads/master. Obviously expansionString part needs some escaping mechanism to allow it to include an unmatched ). -- 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: [RFC/PATCH 07/11] branch: move 'current' check down to the presentation layer
On Wed, Jul 29, 2015 at 6:16 AM, Jacob Keller jacob.kel...@gmail.com wrote: On Tue, Jul 28, 2015 at 1:12 PM, Karthik Nayak karthik@gmail.com wrote: On Tue, Jul 28, 2015 at 6:39 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: We check if given ref is the current branch in print_ref_list(). Move this check to print_ref_item() where it is checked right before printing. This means that the '*' and the different color are coded in C, hence it's not possible to mimick this using git for-each-ref --format I do not consider this as blocking, but I think the ultimate goal should be to allow this, so that all the goodies of git branch can be made available to other ref-listing commands. Not sure what you mean here. He means to make sure that any feature that was in tag, branch, for-each-ref, etc should be available as part of ref-filter and in all of them Maybe he misunderstood code or maybe you misunderstood the comment here? Regards, Jake Ah, I didn't quite understand the first time. Thanks :) -- Regards, Karthik Nayak -- 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: [RFC/PATCH 06/11] branch: roll show_detached HEAD into regular ref_list
On Wed, Jul 29, 2015 at 3:26 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Karthik Nayak karthik@gmail.com writes:
On Tue, Jul 28, 2015 at 6:31 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
-static void show_detached(struct ref_list *ref_list, int maxwidth)
-{
- struct commit *head_commit =
lookup_commit_reference_gently(head_sha1, 1);
-
- if (head_commit is_descendant_of(head_commit,
ref_list-with_commit)) {
I'm not sure what this if was doing, and why you can get rid of it. My
understanding is that with_commit comes from --contains, and in the
previous code the filtering was done at display time (detached HEAD was
not shown if it was not contained in commits specified with --contains).
Eventually, you'll use ref-filter to do this filtering so you won't need
this check at display time.
But am I correct that for a few commits, you ignore --contains on
detached HEAD?
No we don't ignore --contains on detached HEAD.
Since detached HEAD now gets its data from append_ref(). The function
also checks for the --contains option.
Ah, OK. Previously, detached HEAD and branches were completely
different, each having its own if (is_descendant_of(...)), and you're
now using only one in append_ref() before removing it completely in
favor of ref-filter.
That would deserve an explanation for other reviewers I think.
Will include a small explanation in the commit message :)
--
Regards,
Karthik Nayak
--
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: [RFC/PATCH 04/11] ref-filter: add 'ifexists' atom
Junio C Hamano gits...@pobox.com writes: Couldn't think of a better replacer, any suggestions would be welcome :) See below. ... One way to do all of the above is ... Note that is just one way, not the only or not necessarily the best. It certainly is not the easiest, I think. %(if:atom)...%(endif) might be easier to implement. -- 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] git-add-interactive: edit current file in editor
Aside from whether or not this change is desirable, see a few pointers
below to improve the patch...
On Monday, July 27, 2015, Sina Siadat sia...@gmail.com wrote:
Adds a new option 'o' to the 'add -p' command that lets you open and edit the
current file.
Imperative mood: Add a new option...
The existing 'e' mode is used to manually edit the hunk. The new 'o' option
allows you to open and edit the file without having to quit the loop. The
hunks
are updated when the editing is done, and the user will be able to review the
updated hunks. Without this option you would have to quit the loop, edit the
file, and execute 'add -p filename' again.
This descriptive material belongs in the commit message. Good.
I would appreciate it if you could let me know what you think about this
option. I will write more tests if there is any interest at all.
This, however, is commentary, which, while useful as part of the
submission process, does not belong in the commit message. Therefore,
it should be placed below the --- line just before the diffstat.
Thank you. :)
Missing sign-off. See Documentation/SubmittingPatches.
More below...
---
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index deae948..e5dd1c6 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -98,6 +98,12 @@ test_expect_success 'dummy edit works' '
test_cmp expected diff
'
+test_expect_success 'dummy open works' '
+ (echo o; echo a) | git add -p
Some alternatives, which may or may not read better, but at least
avoid a process creation or two:
{ echo o; echo a; } | git add -p
printf %s\n o a | git add -p
printf o\na\n | git add -p
Those are just suggestions; not necessarily a request for change.
+ git diff diff
Style: diff
+ test_cmp expected diff
+'
+
test_expect_success 'setup patch' '
cat patch EOF
@@ -1,1 +1,4 @@
--
2.5.0.rc3.2.g6f9504c.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] ref-filter: fix indentation
Matthieu Moy matthieu@imag.fr writes:
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
This is meant to be applied on top of kn/for-each-ref.
Hmm, as 2.5 is already out and we can rewind 'next' in the coming
couple of days, I am tempted to squash this in instead of applying
it on top.
Thanks.
ref-filter.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 43502a4..3fbbbeb 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -868,8 +868,8 @@ static int ref_filter_handler(const char *refname, const
struct object_id *oid,
struct ref_array_item *ref;
if (flag REF_BAD_NAME) {
- warning(ignoring ref with broken name %s, refname);
- return 0;
+ warning(ignoring ref with broken name %s, refname);
+ return 0;
}
if (*filter-name_patterns
!match_name_as_path(filter-name_patterns, refname))
--
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 4/6] connect: move error check to caller of parse_connect_url
On Wed, Jul 29, 2015 at 11:51 AM, Patrick Steinhardt p...@pks.im wrote:
parse_connect_url() checks if the path component of the URL is
empty and if so causes the program to die. As the function is to
be used at other call sites which do not require this check, move
up the error checking to the existing caller.
Signed-off-by: Patrick Steinhardt p...@pks.im
---
diff --git a/connect.c b/connect.c
index bdbcee4..e8b813d 100644
--- a/connect.c
+++ b/connect.c
@@ -613,9 +613,6 @@ enum protocol parse_connect_url(const char *url_orig,
char **ret_host,
else
path = strchr(end, separator);
- if (!path || !*path)
- die(No path specified. See 'man git-pull' for valid url
syntax);
-
Given that there are several dereferences of 'path' following this bit
of code, won't this change lead to a crash when path==NULL?
/*
* null-terminate hostname and point path to ~ for URL's like this:
*ssh://host.xz/~user/repo
@@ -665,6 +662,9 @@ struct child_process *git_connect(int fd[2], const char
*url,
signal(SIGCHLD, SIG_DFL);
protocol = parse_connect_url(url, hostandport, path);
+ if (!path || !*path)
+ die(No path specified. See 'man git-pull' for valid url
syntax);
+
if ((flags CONNECT_DIAG_URL) (protocol != PROTO_SSH)) {
printf(Diag: url=%s\n, url ? url : NULL);
printf(Diag: protocol=%s\n, prot_name(protocol));
--
2.5.0
--
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] parse-options: align curly braces for all options
Stefan Beller sbel...@google.com writes:
Signed-off-by: Stefan Beller sbel...@google.com
---
The non alignment of white space harmed my feelings for aesthetics more
than it should have.
Hmm, but this does not align curlies for OPT_DATE(),
OPT_EXPIRY_DATE(), etc...
parse-options.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/parse-options.h b/parse-options.h
index c71e9da..08d7818 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -126,7 +126,7 @@ struct option {
#define OPT_BOOL(s, l, v, h)OPT_SET_INT(s, l, v, h, 1)
#define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
NULL, 1}
-#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
+#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG|PARSE_OPT_NONEG,
NULL, (i) }
#define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v),
N_(n), (h) }
#define OPT_STRING(s, l, v, a, h) { OPTION_STRING, (s), (l), (v), (a),
(h) }
--
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] worktree: Correct typo in documentation
Signed-off-by: Johan Sageryd j...@1616.se --- Documentation/git-worktree.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 3387e2f..d9d90b5 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -39,7 +39,7 @@ repository so that they do not get automatically pruned. If a linked working tree is stored on a portable device or network share which is not always mounted, you can prevent its administrative files from -being pruned by creating a file named 'lock' alongside the other +being pruned by creating a file named 'locked' alongside the other administrative files, optionally containing a plain text reason that pruning should be suppressed. See section DETAILS for more information. -- 2.5.0 -- 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: What's cooking in git.git (Jul 2015, #07; Mon, 27)
On Mon, Jul 27, 2015 at 02:23:04PM -0700, Junio C Hamano wrote: * bc/object-id (2015-06-17) 10 commits . remote.c: use struct object_id in many functions . object-id: use struct object_id in struct object . remote.c: use struct object_id in ref_newer() . transport-helper.c: use struct object_id in push_refs_with_export() . connect.c: use struct object_id in get_remote_heads() . remote-curl: use struct object_id in parse_fetch() . fetch-pack: use struct object_id in add_sought_entry_mem() . object_id: convert struct ref to use object_id. . sha1_file: introduce has_object_file() helper . refs: convert some internal functions to use object_id More transition from unsigned char[40] to struct object_id. While GSoC and other topics are actively moving existing code around, this cannot go in; ejected from 'pu'. Is there anything I can do to make this series less painful (e.g. a reroll or such)? If waiting is the best technique, that's fine; I just want to minimize the impact of this change in terms of other code and maintainer burden. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH v4 1/5] refs: Introduce pseudoref and per-worktree ref concepts
On Wed, Jul 29, 2015 at 7:58 PM, David Turner dtur...@twopensource.com wrote: + specially by git. Psuedorefs both have names that are all-caps, s/Psuedo/Pseudo/ ...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
Re: [PATCH] git-am: flag suspiciously old or futuristic commits
On Wed, Jul 29, 2015 at 3:20 PM, Stefan Beller sbel...@google.com wrote: On Wed, Jul 29, 2015 at 3:01 PM, Paul Gortmaker paul.gortma...@windriver.com wrote: The linux kernel repository has some commits in it with dates from the year 1970 and also 2030 (and possibly others). We probably shouldi warn people when the dates look suspect. For commits in the future, note that a committer in Australia could commit on New Years Day, and send it to a maintainer in North America and that would trip the notification on the maintainer's New Years Eve. But that is unlikely, and the note is still correct; that the commit is from a future year. For commits in the past, I chose a somewhat arbitrary 30 year limit, which will allow stuff from post 1985; the thought being that someone might want to import an old repo into git from some other SCM. We could alternatively set it to 5, which would then catch computers with a dead CMOS battery, at the risk of pestering the hypothetical museum curator of old bits. Sample output: paul@builder:~/git/linux-head$ grep Date: *patch future.patch:Date: Sat, 18 Jul 2037 21:22:19 -0400 past.patch:Date: Sat, 18 Jul 1977 21:22:19 -0400 paul@builder:~/git/linux-head$ git am future.patch note: commit is from future year 2037. Applying: arch/sh: make heartbeat driver explicitly non-modular paul@builder:~/git/linux-head$ git reset --hard HEAD~ /dev/null paul@builder:~/git/linux-head$ git am past.patch note: commit is from implausibly old year 1977. Applying: arch/sh: make heartbeat driver explicitly non-modular paul@builder:~/git/linux-head$ Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com --- git-am.sh | 15 +++ 1 file changed, 15 insertions(+) diff --git a/git-am.sh b/git-am.sh [+cc paul tan, who rewrote am in c as a GSoC project.] index 3af351ffaaf3..ff6deb8047a4 100755 --- a/git-am.sh +++ b/git-am.sh @@ -766,6 +766,21 @@ To restore the original branch and stop patching run \\$cmdline --abort\. stop_here $this fi + if test -n $GIT_AUTHOR_DATE + then + THIS_YEAR=`date +%Y` + TOO_OLD=$(expr $THIS_YEAR - 30) + TOO_NEW=$(expr $THIS_YEAR + 1) + GIT_AUTHOR_YEAR=`date -d $GIT_AUTHOR_DATE +%Y` Would it make sense to not operate on year but on unix time, so the problem you mentioned in the commit message goes away? Another thought: Having this check in am seems a bit arbitrary to me (or rather workflow adapted ;) as we could also check in commit or pull (not sure if I actually mean the fetch or merge thereof) I think this makes most sense in am, as it is most likely to show up, in my mind, due to a format-patch mistake. If we do it during pull, would we just warn? how would we reject commits? that doesn't really fit.. We can't do it during commit, as obviously the broken machine will likely not be able to notice it at all.. We could check remotes during push but that doesn't solve this either.. Maybe just emitting a warning during a fetch or am (since am doesn't use pull) would make the most sense? I don't think we can reject things when doing a fetch though, but we could warn. Regards, Jake -- 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
Confused about sparse vs untracked-cache
I'm looking at dir.c, and there's a bit I'm confused about:
prep_exclude() says:
/*
* .. and .gitignore does not exist before
* (i.e. null exclude_sha1 and skip_worktree is
* not set). Then we can skip loading .gitignore,
* which would result in ENOENT anyway.
* skip_worktree is taken care in read_directory()
*/
!is_null_sha1(untracked-exclude_sha1))) {
That skip_worktree is taken care in read_directory() appears to be
referring to this bit of validate_untracked_cache():
/*
* An optimization in prep_exclude() does not play well with
* CE_SKIP_WORKTREE. It's a rare case anyway, if a single
* entry has that bit set, disable the whole untracked cache.
*/
for (i = 0; i active_nr; i++)
if (ce_skip_worktree(active_cache[i]))
return NULL;
I'm confused about why skip_worktree needs to be unset. When I comment
out the second snippet, all the tests still pass. What was the reason
behind that condition? Is it really necessary?
--
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: [RFC/PATCH 02/11] ref-filter: add 'colornext' atom
On Wed, Jul 29, 2015 at 2:30 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Eric Sunshine sunsh...@sunshineco.com writes:
Also, please explain here and in the commit message why this highly
specialized colorizer ('colornext'), is needed even though a more
general purpose one ('color') is already available.
It is needed in the current form to allow
%(colornext:blue)%(ifexists:[%s]) to color only the replacement of %s
and not the [].
But I now think that this would be more elegantly solved by Junio's
%(if) %(endif) idea:
%(if:atom) [ %(color:blue)%(atom)%(color:reset) ] %(endif)
(I added spaces for clarity)
I agree, this style seems a lot more elegant and expressive while much
easier to understand. Same for doing this to the alignment atoms and
such as it solves the same problem(s) there.
I can't speak to how easy it is to implement tho.
Regards,
Jake
--
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/6] refs: add ref_type function
On Tue, Jul 28, 2015 at 2:12 PM, David Turner dtur...@twopensource.com wrote:
Add a function ref_type, which categorizes refs as per-worktree,
pseudoref, or normal ref.
Later, we will use this in refs.c to treat pseudorefs specially.
Alternate ref backends may use it to treat both pseudorefs and
per-worktree refs differently.
Signed-off-by: David Turner dtur...@twopensource.com
---
diff --git a/refs.c b/refs.c
index 0b96ece..553ae8b 100644
--- a/refs.c
+++ b/refs.c
@@ -2848,6 +2848,35 @@ static int delete_ref_loose(struct ref_lock *lock, int
flag, struct strbuf *err)
return 0;
}
+static int is_per_worktree_ref(const char *refname)
+{
+ return !strcmp(refname, HEAD);
+}
+
+static int is_pseudoref_syntax(const char *refname)
+{
+ const char *c;
+
+ if (strchr(refname, '/'))
+ return 0;
Isn't this redundant? Won't the below for-loop already return 0 if it
encounters a slash?
+ for (c = refname; *c; ++c) {
Style: c++
+ if (!isupper(*c) *c != '-' *c != '_')
+ return 0;
+ }
+
+ return 1;
+}
+
+enum ref_type ref_type(const char *refname)
+{
+ if (is_per_worktree_ref(refname))
+ return REF_TYPE_PER_WORKTREE;
+ if (is_pseudoref_syntax(refname))
+ return REF_TYPE_PSEUDOREF;
+ return REF_TYPE_NORMAL;
+}
--
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 4/6] pseudorefs: create and use pseudoref update and delete functions
On Tue, Jul 28, 2015 at 2:12 PM, David Turner dtur...@twopensource.com wrote:
Pseudorefs should not be updated through the ref transaction
API, because alternate ref backends still need to store pseudorefs
in GIT_DIR (instead of wherever they store refs). Instead,
change update_ref and delete_ref to call pseudoref-specific
functions.
Signed-off-by: David Turner dtur...@twopensource.com
---
diff --git a/refs.c b/refs.c
index 553ae8b..2bd6aa6 100644
--- a/refs.c
+++ b/refs.c
@@ -2877,12 +2877,87 @@ enum ref_type ref_type(const char *refname)
+static int delete_pseudoref(const char *pseudoref, const unsigned char
*old_sha1)
+{
+ static struct lock_file lock;
+ const char *filename;
+
+ filename = git_path(%s, pseudoref);
+
+ if (old_sha1 !is_null_sha1(old_sha1)) {
+ int fd;
+ unsigned char actual_old_sha1[20];
+
+ fd = hold_lock_file_for_update(lock, filename,
+ LOCK_DIE_ON_ERROR);
+ if (fd 0)
+ die_errno(_(Could not open '%s' for writing),
filename);
+ read_ref(pseudoref, actual_old_sha1);
+ if (hashcmp(actual_old_sha1, old_sha1)) {
+ warning(Unexpected sha1 when deleting %s,
pseudoref);
+ return -1;
Does this need to release the lock file before returning?
+ }
+
+ unlink(filename);
+ rollback_lock_file(lock);
+ } else {
+ unlink(filename);
+ }
+
+ return 0;
+}
--
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] userdiff: add support for Fountain documents
Hi again! Where's this at? Your last regex looks perfect to me: ^((\\.[^.]|(int|ext|est|int\\.?/ext|i/e)[. ]).*)$ Do you need anything else from me? Thanks, Zoë.-- 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: [RFC/PATCH 06/11] branch: roll show_detached HEAD into regular ref_list
Karthik Nayak karthik@gmail.com writes:
On Tue, Jul 28, 2015 at 6:31 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
-static void show_detached(struct ref_list *ref_list, int maxwidth)
-{
- struct commit *head_commit =
lookup_commit_reference_gently(head_sha1, 1);
-
- if (head_commit is_descendant_of(head_commit,
ref_list-with_commit)) {
I'm not sure what this if was doing, and why you can get rid of it. My
understanding is that with_commit comes from --contains, and in the
previous code the filtering was done at display time (detached HEAD was
not shown if it was not contained in commits specified with --contains).
Eventually, you'll use ref-filter to do this filtering so you won't need
this check at display time.
But am I correct that for a few commits, you ignore --contains on
detached HEAD?
No we don't ignore --contains on detached HEAD.
Since detached HEAD now gets its data from append_ref(). The function
also checks for the --contains option.
Ah, OK. Previously, detached HEAD and branches were completely
different, each having its own if (is_descendant_of(...)), and you're
now using only one in append_ref() before removing it completely in
favor of ref-filter.
That would deserve an explanation for other reviewers I think.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: [RFC/PATCH 07/11] branch: move 'current' check down to the presentation layer
Karthik Nayak karthik@gmail.com writes: On Tue, Jul 28, 2015 at 6:39 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: We check if given ref is the current branch in print_ref_list(). Move this check to print_ref_item() where it is checked right before printing. This means that the '*' and the different color are coded in C, hence it's not possible to mimick this using git for-each-ref --format I do not consider this as blocking, but I think the ultimate goal should be to allow this, so that all the goodies of git branch can be made available to other ref-listing commands. Not sure what you mean here. What you already know, but probably badly explained ;-). Eventually, the output of git branch should correspond to a format string (so git branch would be almost an alias for git for-each-ref refs/heads/ --format '...'). Internally, this would mean using show_ref_array_item instead of print_ref_item. This is what you managed to do for git tag. You already identified one difficulty with sha1 alignment in git branch -v. I'm pointing out another which is that displaying the * in front of the current branch is currently not possible with a format string. You would need an atom like %(displayAStarIfTheBranchIsTheCurrentOne) (for which you'd need to find a short-and-sweet name ;-) ). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: Log messages beginning # and git rebase -i
Junio C Hamano gits...@pobox.com writes: OK. So the proposal on the table is that a backslash at the beginning of a line is stripped. Yes. Stripping part should look like this. Thanks. To make it work for things like git commit --amend, you would need to prefix any line that comes from the payload that begins with the core.commentchar or a backslash with a backslash. That's it, probably the hardest part. No Git time budget for now, but I'm adding this here in case I get time or a student to work on it: http://git.wiki.kernel.org/index.php/SmallProjectsIdeas -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: Log messages beginning # and git rebase -i
On Wed, Jul 29, 2015 at 12:48 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: If the user wants whatever she types in the resulting commit literally, there is the --cleanup=choice option, no? $ GIT_EDITOR=touch git commit --cleanup=verbatim [detached HEAD 1b136a7] # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # HEAD detached from 5e70007 # Changes to be committed: # modified: foo.txt # # Changes not staged for commit : # modified: foo.txt # # Untracked files: # last-synchro.txt # 1 file changed, 1 insertion(+), 1 deletion(-) You really don't want that in day-to-day use. How about --cleanup=scissors? The chance that you have the same cut line in your commit message is really low, compared to having comment characters. -- 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: Log messages beginning # and git rebase -i
Thank you for looking into this. -- Ed Avis e...@waniasset.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
git svn clone fails
Hello guys, we have to turn some projects managed by Subversion into a GIT-based solution. However the conversion unfortunately fails with an error: == % == [...] Found branch parent: (refs/remotes/origin/tags/v_2.1.1) 450fa2c84a8fc087a2a66e5fb3d6d22096671f81 Following parent with do_switch M changes.xml M pom.xml couldn't truncate file at /usr/lib64/perl5/vendor_perl/5.20.2/Git.pm line 1325. $ git --version git version 2.5.0 == % == Apart from the line number, the same error occurs also with GIT version 2.4.6 and 2.3.6 (latest stable version in Gentoo). The command to create the GIT repository was: == % == $ git svn clone http://websvn/svn/essvn/standard/java-commons/lang -s --no- metadata --preserve-empty-dirs -A ~/tmp/authors.txt commons-lang == % == Note, that the command succeeds even for 2.4.6 if we drop the --preserve- empty-dirs option - at the cost of some empty directories which are used to trigger profiles for Maven; i.e. without those we cannot reproduce released versions. Cheers, Jörg -- 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: [RFC/PATCH 09/11] branch.c: use 'ref-filter' data structures
Karthik Nayak karthik@gmail.com writes:
On Tue, Jul 28, 2015 at 7:18 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Christian Couder christian.cou...@gmail.com writes:
On Tue, Jul 28, 2015 at 8:56 AM, Karthik Nayak karthik@gmail.com
wrote:
+static void ref_array_append(struct ref_array *array, const char *refname)
+{
+ size_t len = strlen(refname);
+ struct ref_array_item *ref = xcalloc(1, sizeof(struct
ref_array_item) + len + 1);
+ memcpy(ref-refname, refname, len);
+ ref-refname[len] = '\0';
This looks very much like new_ref_array_item, except that the later also
takes an objectname parameter. I find it suspicious that you leave the
objectname field uninitialized.
Well the objectname is not required here, the idea is to retain the way
branch.c
works.
Why is this code not calling new_ref_array_item?
Because no objectname is there.
You do have the object_id in the scope of the call-site, so why not use
it?
(Well, in any case, do as you think is best, it's temporary throw-away
code, we shouldn't loose too much time polishing it)
The function disapears in the next commit, but I also think that this
function deserves to exist in ref-filter.{c,h} and remain after the end
of the series.
Why though? I don't see the need when new_ref_array_item() is present.
OK, if the function is specificly for append an item but leave the
objectname uninitialized, it's too specific to be useful somewhere
else. But then, make it more explicit in the name of the function and/or
in a comment.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: Log messages beginning # and git rebase -i
On Wed, Jul 29, 2015 at 7:17 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Duy Nguyen pclo...@gmail.com writes: On Wed, Jul 29, 2015 at 12:48 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: If the user wants whatever she types in the resulting commit literally, there is the --cleanup=choice option, no? $ GIT_EDITOR=touch git commit --cleanup=verbatim [detached HEAD 1b136a7] # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # HEAD detached from 5e70007 # Changes to be committed: # modified: foo.txt # # Changes not staged for commit : # modified: foo.txt # # Untracked files: # last-synchro.txt # 1 file changed, 1 insertion(+), 1 deletion(-) You really don't want that in day-to-day use. How about --cleanup=scissors? I can read this in two different ways: 1) Keeping git as-is and suggest users to use --cleanup=scissors This has the same problem as --cleanup=verbatim: it doesn't work as-is since Git doesn't insert the scissors. You can hack around it by adding them by yourself when you need it, but it's really not convenient. You have to anticipate that you're going to require a # and call commit with --cleanup=scissors, add the scissors. And repeat it if you need to commit --amend. 2) Modify Git to add scissors by default, and use --cleanup=scissors by default. This is actually more or less what SVN does: it inserts a line --This line, and those below, will be ignored--, and the equivalent of what Git adds as comments in the template is inserted below this line. I don't think option 1) is good. The fact that we have the --cleanup= option shouldn't serve as an excuse to do nothing. I'd be fine with option 2), but I find it much more intrusive than to allow a simple backslash-escaping as I suggest. auto backslashing could cause some annoyance. Emacs supports rearranging a paragraph to fit in a fixed text column. This generated backslash may be moved around, no longer at the beginning of the line, and it will remain in the commit message. I don't know how popular this feature is outside emacs. Having said that, even scissors has its own (and probably bigger) problem: when you commit after conflict resolution, git inserts a Conflicts: paragraph, prepended by core.commentChar. With default settings, it serves as a reminder, but will be automatically stripped. With scissors, it stays by default because it's placed before the scissor line. -- 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 v3 2/6] notes: replace pseudorefs with real refs
On Wed, Jul 29, 2015 at 7:01 AM, Junio C Hamano gits...@pobox.com wrote: Johan Herland jo...@herland.net writes: I believe it is a bad compromise. It complicates the code, and it provides a concurrent notes merges that is unnecessarily tied to (and dependent on) worktrees. For example, if I wanted to perform N concurrent notes merges in a project that happens to have a huge worktree, I would now have to create N copies of the huge worktree... Who said worktree has to be populated? You should be able to have an absolutely empty checkout just like clone --no-checkout. IMHO that's an insane workaround that only serves to highlight the conceptual problems of binding notes merges (as they are implemented today) to worktrees. I'm much more excited about Michael's idea of formalising the concept of a notes tree checkout (although as already discussed, checking out a notes tree is different from checking out a regular tree). Then, a notes merge (as you already suggested) should be implemented by creating a linked worktree whose HEAD is the notes ref being merged into. I believe there are potential complications here as well (where a notes-merge might behave differently from a regular merge), but those should be solvable. But, whatever. This is unrelated to David's current effort, and I don't want to hold that up, so please move along, nothing to see here. ...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
Re: [PATCH] worktree: Correct typo in documentation
On Wed, Jul 29, 2015 at 7:03 PM, Johan Sageryd j...@1616.se wrote: Signed-off-by: Johan Sageryd j...@1616.se --- diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 3387e2f..d9d90b5 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -39,7 +39,7 @@ repository so that they do not get automatically pruned. If a linked working tree is stored on a portable device or network share which is not always mounted, you can prevent its administrative files from -being pruned by creating a file named 'lock' alongside the other +being pruned by creating a file named 'locked' alongside the other administrative files, optionally containing a plain text reason that pruning should be suppressed. See section DETAILS for more information. Thanks for the patch. This is already fixed in the 'next' branch by 2e73ab6 (Documentation/git-worktree: fix incorrect reference to file locked, 2015-07-20). -- 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: Log messages beginning # and git rebase -i
Duy Nguyen pclo...@gmail.com writes: On Wed, Jul 29, 2015 at 12:48 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: If the user wants whatever she types in the resulting commit literally, there is the --cleanup=choice option, no? $ GIT_EDITOR=touch git commit --cleanup=verbatim [detached HEAD 1b136a7] # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # HEAD detached from 5e70007 # Changes to be committed: # modified: foo.txt # # Changes not staged for commit : # modified: foo.txt # # Untracked files: # last-synchro.txt # 1 file changed, 1 insertion(+), 1 deletion(-) You really don't want that in day-to-day use. How about --cleanup=scissors? I can read this in two different ways: 1) Keeping git as-is and suggest users to use --cleanup=scissors This has the same problem as --cleanup=verbatim: it doesn't work as-is since Git doesn't insert the scissors. You can hack around it by adding them by yourself when you need it, but it's really not convenient. You have to anticipate that you're going to require a # and call commit with --cleanup=scissors, add the scissors. And repeat it if you need to commit --amend. 2) Modify Git to add scissors by default, and use --cleanup=scissors by default. This is actually more or less what SVN does: it inserts a line --This line, and those below, will be ignored--, and the equivalent of what Git adds as comments in the template is inserted below this line. I don't think option 1) is good. The fact that we have the --cleanup= option shouldn't serve as an excuse to do nothing. I'd be fine with option 2), but I find it much more intrusive than to allow a simple backslash-escaping as I suggest. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
