Re: [PATCH v10 20/20] untracked-cache: config option
On Fri, May 13, 2016 at 3:20 AM, David Turner wrote:
> Add a config option to populate the untracked cache.
>
> For installations that have centrally-managed configuration, it's
> easier to set a config once than to run update-index on every
> repository.
This sounds like the job for core.untrackedCache. It populates after
reading the index though (in post_read_index_from) not writing, but I
think it accomplishes the same thing.
> Signed-off-by: David Turner
> ---
> Documentation/config.txt | 4
> read-cache.c | 7 ++-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 385ea66..c7b76ef 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1848,6 +1848,10 @@ imap::
> The configuration variables in the 'imap' section are described
> in linkgit:git-imap-send[1].
>
> +index.adduntrackedcache::
> + Automatically populate the untracked cache whenever the index
> + is written.
> +
> index.addwatchmanextension::
> Automatically add the watchman extension to the index whenever
> it is written.
> diff --git a/read-cache.c b/read-cache.c
> index 22c64d0..4a1cccf 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2472,7 +2472,7 @@ static int do_write_index(struct index_state *istate,
> int newfd,
> int entries = istate->cache_nr;
> struct stat st;
> struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
> - int watchman = 0;
> + int watchman = 0, untracked = 0;
> uint64_t start = getnanotime();
>
> for (i = removed = extended = 0; i < entries; i++) {
> @@ -2502,6 +2502,11 @@ static int do_write_index(struct index_state *istate,
> int newfd,
> !the_index.last_update)
> the_index.last_update = xstrdup("");
>
> + if (!git_config_get_bool("index.adduntrackedcache", &untracked) &&
> + untracked &&
> + !istate->untracked)
> + add_untracked_cache(&the_index);
> +
> hdr_version = istate->version;
>
> hdr.hdr_signature = htonl(CACHE_SIGNATURE);
> --
> 2.4.2.767.g62658d5-twtrsrc
>
--
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: [RFC PATCH 0/4] pathspec labels [WAS: submodule groups]
On Fri, May 13, 2016 at 7:19 AM, Stefan Beller wrote: > After some fruitful discussion[1] in which Junio suggested trying a very > different route[2] that is more general and not submodule related, I > considered > doing a mock for this. > > This lets you label arbitrary pathspecs, e.g. in git.git we may want to have: > > t/t[0-9]*.sh label=tests > > such that > > $ git log --author=Beller ":(label=tests)" > > would show all commits in which I touched tests. > > This has suprisingly few lines of code, as the first 3 patches are > refactoring. > The actual new feature is in the last patch. > > This would solve the submodule issues I want to solve, as I can produce a > .gitattributes like: > > ./submodule1 label=default > ./submodule2 label=default > ./submodule3 label=optional-feature1 > > and then I'd instruct the users to clone like this: > > git clone .. superproject > cd superproject > git submodule update --init :(label:default) > > The second part of the submodule series to collapse these three commands > will come as an extra series later, then. Yeah. I can see the use of this, even without submodules. > What annoys me here: > Attributes can only be set once at the moment, there is no way to collect all > attributes. If we'd have such a collection feature we would be able to > configure this: > > *.[ch] label=C,code > contrib/** label=oldstuff Instead of putting everything in under the same attribute name "label", make one attribute per label? Would this work? *.[ch] c-group code-group And the pathspec magic name can be simply "attr", any git attribute can be used with it, e.g. :(attr:c-group) > and run this: > > git status ":(label:C oldstuff)" > > which would be the equivalent to > > contrib/**.[ch] > > as in this proposed implementation the labels which are given within one > pathspec item are logical AND. To get the logical OR, you'd have to give > multiple > pathspec items, i.e. ":(label:C)" ":(label:oldstuff)" It's even better if pathspec does not have to deal with combination logic. Can attr macros deal with that (or if it can't at the moment, can we improve it)? -- 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: 'git diff-index' doesn't honor the 'diff.algorithm' variable
Hi Junio, On 05/14/2016 09:40 PM, Junio C Hamano wrote: The variable belongs to UI config, meant for Porcelain "git diff", together with things like "diff.color", "diff.context", etc. OK, that makes sense. You might want to fix the man page, though, it says, like the 'git diff' one, "For instance, if you configured diff.algorithm variable to a non-default value and want to use the default one, then you have to use --diff-algorithm=default option.". A script that calls diff-index, if it wants to honor end-users' UI config variables, is allowed to use 'git config' to read them and turn them into appropriate command line options. e.g. algo=$(git config diff.algorithm) case "$algo" in minimal|histogram|patience) algo=--$algo ;; *) algo= ;; esac ... git diff-index $algo ... other args ... or something like that. Thanks, but we don't distribute any custom Git porcelains with Emacs. We usually can't rely on bash being available either. Doing an extra process call from Emacs for this niche a feature doesn't seem like a great idea either. To clarify, the problem is that `M-x vc-diff' doesn't honor the diff.algorithm option. I'll have to see why we using 'git diff-index' there directly. Maybe we could switch to 'git diff'. -- 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 00/17] Port branch.c to use ref-filter's printing options
This is part of unification of the commands 'git tag -l, git branch -l and git for-each-ref'. This ports over branch.c to use ref-filter's printing options. Initially posted here: $(gmane/279226). It was decided that this series would follow up after refactoring ref-filter parsing mechanism, which is now merged into master (9606218b32344c5c756f7c29349d3845ef60b80c). v1 can be found here: $(gmane/288342) v2 can be found here: $(gmane/288863) v3 can be found here: $(gmane/290299) v4 can be found here: $(gmane/291106) v5b can be found here: $(gmane/292467) Changes in this version: 1. Rebased on top of f307218 (t6302: simplify non-gpg cases). This patch ensures that we only use signed tags when GPG is installed and effectively gets rid of the bug introduced by 01/17 (ref-filter: implement %(if), %(then), and %(else) atoms)[1]. [1]: $(gmane/293901) Karthik Nayak (17): ref-filter: implement %(if), %(then), and %(else) atoms ref-filter: include reference to 'used_atom' within 'atom_value' ref-filter: implement %(if:equals=) and %(if:notequals=) ref-filter: modify "%(objectname:short)" to take length ref-filter: move get_head_description() from branch.c ref-filter: introduce format_ref_array_item() ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams ref-filter: add support for %(upstream:track,nobracket) ref-filter: make "%(symref)" atom work with the ':short' modifier ref-filter: introduce refname_atom_parser_internal() ref-filter: introduce symref_atom_parser() and refname_atom_parser() ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal() ref-filter: add `:dir` and `:base` options for ref printing atoms ref-filter: allow porcelain to translate messages in the output branch, tag: use porcelain output branch: use ref-filter printing APIs branch: implement '--format' option Documentation/git-branch.txt | 7 +- Documentation/git-for-each-ref.txt | 87 +-- builtin/branch.c | 277 +++--- builtin/tag.c | 2 + ref-filter.c | 456 +++-- ref-filter.h | 7 + t/t3203-branch-output.sh | 16 +- t/t6040-tracking-info.sh | 2 +- t/t6300-for-each-ref.sh| 73 +- t/t6302-for-each-ref-filter.sh | 94 10 files changed, 726 insertions(+), 295 deletions(-) -- 2.8.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 03/17] ref-filter: implement %(if:equals=) and %(if:notequals=)
Implement %(if:equals=) wherein the if condition is only
satisfied if the value obtained between the %(if:...) and %(then) atom
is the same as the given ''.
Similarly, implement (if:notequals=) wherein the if condition
is only satisfied if the value obtained between the %(if:...) and
%(then) atom is differnt from the given ''.
This is done by introducing 'if_atom_parser()' which parses the given
%(if) atom and then stores the data in used_atom which is later passed
on to the used_atom of the %(then) atom, so that it can do the required
comparisons.
Add tests and Documentation for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 3 +++
ref-filter.c | 43 +-
t/t6302-for-each-ref-filter.sh | 18
3 files changed, 59 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index aea37b2..58dc9e7 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -152,6 +152,9 @@ if::
evaluating the string before %(then), this is useful when we
use the %(HEAD) atom which prints either "*" or " " and we
want to apply the 'if' condition only on the 'HEAD' ref.
+ Append ":equals=" or ":notequals=" to compare
+ the value between the %(if:...) and %(then) atoms with the
+ given string.
In addition to the above, for commit and tag objects, the header
field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/ref-filter.c b/ref-filter.c
index 12e646c..857a8b5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -22,6 +22,8 @@ struct align {
};
struct if_then_else {
+ const char *if_equals,
+ *not_equals;
unsigned int then_atom_seen : 1,
else_atom_seen : 1,
condition_satisfied : 1;
@@ -49,6 +51,10 @@ static struct used_atom {
enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG,
C_SUB } option;
unsigned int nlines;
} contents;
+ struct {
+ const char *if_equals,
+ *not_equals;
+ } if_then_else;
enum { O_FULL, O_SHORT } objectname;
} u;
} *used_atom;
@@ -169,6 +175,19 @@ static void align_atom_parser(struct used_atom *atom,
const char *arg)
string_list_clear(¶ms, 0);
}
+static void if_atom_parser(struct used_atom *atom, const char *arg)
+{
+ if (!arg)
+ return;
+ else if (skip_prefix(arg, "equals=", &atom->u.if_then_else.if_equals))
+;
+ else if (skip_prefix(arg, "notequals=",
&atom->u.if_then_else.not_equals))
+ ;
+ else
+ die(_("unrecognized %%(if) argument: %s"), arg);
+}
+
+
static struct {
const char *name;
cmp_type cmp_type;
@@ -209,7 +228,7 @@ static struct {
{ "color", FIELD_STR, color_atom_parser },
{ "align", FIELD_STR, align_atom_parser },
{ "end" },
- { "if" },
+ { "if", FIELD_STR, if_atom_parser },
{ "then" },
{ "else" },
};
@@ -410,6 +429,9 @@ static void if_atom_handler(struct atom_value *atomv,
struct ref_formatting_stat
struct ref_formatting_stack *new;
struct if_then_else *if_then_else = xcalloc(sizeof(struct
if_then_else), 1);
+ if_then_else->if_equals = atomv->atom->u.if_then_else.if_equals;
+ if_then_else->not_equals = atomv->atom->u.if_then_else.not_equals;
+
push_stack_element(&state->stack);
new = state->stack;
new->at_end = if_then_else_handler;
@@ -441,10 +463,17 @@ static void then_atom_handler(struct atom_value *atomv,
struct ref_formatting_st
die(_("format: %%(then) atom used after %%(else)"));
if_then_else->then_atom_seen = 1;
/*
-* If there exists non-empty string between the 'if' and
-* 'then' atom then the 'if' condition is satisfied.
+* If the 'equals' or 'notequals' attribute is used then
+* perform the required comparison. If not, only non-empty
+* strings satisfy the 'if' condition.
*/
- if (cur->output.len && !is_empty(cur->output.buf))
+ if (if_then_else->if_equals) {
+ if (!strcmp(if_then_else->if_equals, cur->output.buf))
+ if_then_else->condition_satisfied = 1;
+ } else if (if_then_else->not_equals) {
+ if (strcmp(if_then_else->not_equals, cur->output.buf))
+ if_then_else->condition_satisfied = 1;
+ } else if (cur->output.len && !is_empty(cur->output.buf))
if_then_else->condition_satisfied = 1;
strbuf_reset(&cur->output);
}
@@ -1137,7 +1166,11 @@ static void populate_value(struct ref_array_item *ref)
} else i
[PATCH v6 05/17] ref-filter: move get_head_description() from branch.c
Move the implementation of get_head_description() from branch.c to
ref-filter. This gives a description of the HEAD ref if called. This
is used as the refname for the HEAD ref whenever the
FILTER_REFS_DETACHED_HEAD option is used. Make it public because we
need it to calculate the length of the HEAD refs description in
branch.c:calc_maxwidth() when we port branch.c to use ref-filter
APIs.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/branch.c | 33 -
ref-filter.c | 38 --
ref-filter.h | 2 ++
3 files changed, 38 insertions(+), 35 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 2ecde53..141168d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -361,39 +361,6 @@ static void add_verbose_info(struct strbuf *out, struct
ref_array_item *item,
strbuf_release(&subject);
}
-static char *get_head_description(void)
-{
- struct strbuf desc = STRBUF_INIT;
- struct wt_status_state state;
- memset(&state, 0, sizeof(state));
- wt_status_get_state(&state, 1);
- if (state.rebase_in_progress ||
- state.rebase_interactive_in_progress)
- strbuf_addf(&desc, _("(no branch, rebasing %s)"),
- state.branch);
- else if (state.bisect_in_progress)
- strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
- state.branch);
- else if (state.detached_from) {
- if (state.detached_at)
- /* TRANSLATORS: make sure this matches
- "HEAD detached at " in wt-status.c */
- strbuf_addf(&desc, _("(HEAD detached at %s)"),
- state.detached_from);
- else
- /* TRANSLATORS: make sure this matches
- "HEAD detached from " in wt-status.c */
- strbuf_addf(&desc, _("(HEAD detached from %s)"),
- state.detached_from);
- }
- else
- strbuf_addstr(&desc, _("(no branch)"));
- free(state.branch);
- free(state.onto);
- free(state.detached_from);
- return strbuf_detach(&desc, NULL);
-}
-
static void format_and_print_ref_item(struct ref_array_item *item, int
maxwidth,
struct ref_filter *filter, const char
*remote_prefix)
{
diff --git a/ref-filter.c b/ref-filter.c
index 7d3af1c..fcb3353 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,7 @@
#include "utf8.h"
#include "git-compat-util.h"
#include "version.h"
+#include "wt-status.h"
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
@@ -1077,6 +1078,37 @@ static void fill_remote_ref_details(struct used_atom
*atom, const char *refname,
*s = refname;
}
+char *get_head_description(void)
+{
+ struct strbuf desc = STRBUF_INIT;
+ struct wt_status_state state;
+ memset(&state, 0, sizeof(state));
+ wt_status_get_state(&state, 1);
+ if (state.rebase_in_progress ||
+ state.rebase_interactive_in_progress)
+ strbuf_addf(&desc, _("(no branch, rebasing %s)"),
+ state.branch);
+ else if (state.bisect_in_progress)
+ strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
+ state.branch);
+ else if (state.detached_from) {
+ /* TRANSLATORS: make sure these match _("HEAD detached at ")
+ and _("HEAD detached from ") in wt-status.c */
+ if (state.detached_at)
+ strbuf_addf(&desc, _("(HEAD detached at %s)"),
+ state.detached_from);
+ else
+ strbuf_addf(&desc, _("(HEAD detached from %s)"),
+ state.detached_from);
+ }
+ else
+ strbuf_addstr(&desc, _("(no branch)"));
+ free(state.branch);
+ free(state.onto);
+ free(state.detached_from);
+ return strbuf_detach(&desc, NULL);
+}
+
/*
* Parse the object referred by ref, and grab needed value.
*/
@@ -1116,9 +1148,11 @@ static void populate_value(struct ref_array_item *ref)
name++;
}
- if (starts_with(name, "refname"))
+ if (starts_with(name, "refname")) {
refname = ref->refname;
- else if (starts_with(name, "symref"))
+ if (ref->kind & FILTER_REFS_DETACHED_HEAD)
+ refname = get_head_description();
+ } else if (starts_with(name, "symref"))
refname = ref->symref ? ref->symref : "";
else if (starts_with(name, "upstream")) {
const char *branch_name;
diff --git a/re
[PATCH v6 06/17] ref-filter: introduce format_ref_array_item()
To allow column display, we will need to first render the output in a
string list to allow print_columns() to compute the proper size of
each column before starting the actual output. Introduce the function
format_ref_array_item() that does the formatting of a ref_array_item
to an strbuf.
show_ref_array_item() is kept as a convenience wrapper around it which
obtains the strbuf and prints it the standard output.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
ref-filter.c | 16
ref-filter.h | 3 +++
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index fcb3353..da2b9ee 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1813,10 +1813,10 @@ static void append_literal(const char *cp, const char
*ep, struct ref_formatting
}
}
-void show_ref_array_item(struct ref_array_item *info, const char *format, int
quote_style)
+void format_ref_array_item(struct ref_array_item *info, const char *format,
+ int quote_style, struct strbuf *final_buf)
{
const char *cp, *sp, *ep;
- struct strbuf *final_buf;
struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
state.quote_style = quote_style;
@@ -1846,9 +1846,17 @@ void show_ref_array_item(struct ref_array_item *info,
const char *format, int qu
}
if (state.stack->prev)
die(_("format: %%(end) atom missing"));
- final_buf = &state.stack->output;
- fwrite(final_buf->buf, 1, final_buf->len, stdout);
+ strbuf_addbuf(final_buf, &state.stack->output);
pop_stack_element(&state.stack);
+}
+
+void show_ref_array_item(struct ref_array_item *info, const char *format, int
quote_style)
+{
+ struct strbuf final_buf = STRBUF_INIT;
+
+ format_ref_array_item(info, format, quote_style, &final_buf);
+ fwrite(final_buf.buf, 1, final_buf.len, stdout);
+ strbuf_release(&final_buf);
putchar('\n');
}
diff --git a/ref-filter.h b/ref-filter.h
index 4aea594..0014b92 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -98,6 +98,9 @@ int parse_ref_filter_atom(const char *atom, const char *ep);
int verify_ref_format(const char *format);
/* Sort the given ref_array as per the ref_sorting provided */
void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
+/* Based on the given format and quote_style, fill the strbuf */
+void format_ref_array_item(struct ref_array_item *info, const char *format,
+ int quote_style, struct strbuf *final_buf);
/* Print the ref using the given format and quote_style */
void show_ref_array_item(struct ref_array_item *info, const char *format, int
quote_style);
/* Callback function for parsing the sort option */
--
2.8.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 08/17] ref-filter: add support for %(upstream:track,nobracket)
Add support for %(upstream:track,nobracket) which will print the
tracking information without the brackets (i.e. "ahead N, behind M").
This is needed when we port branch.c to use ref-filter's printing APIs.
Add test and documentation for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 8 +++--
ref-filter.c | 67 +-
t/t6300-for-each-ref.sh| 2 ++
3 files changed, 51 insertions(+), 26 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index ba27809..7f0cb19 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -118,9 +118,11 @@ upstream::
`refname` above. Additionally respects `:track` to show
"[ahead N, behind M]" and `:trackshort` to show the terse
version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
- or "=" (in sync). Has no effect if the ref does not have
- tracking information associated with it. `:track` also prints
- "[gone]" whenever unknown upstream ref is encountered.
+ or "=" (in sync). `:track` also prints "[gone]" whenever
+ unknown upstream ref is encountered. Append `:track,nobracket`
+ to show tracking information without brackets (i.e "ahead N,
+ behind M"). Has no effect if the ref does not have tracking
+ information associated with it.
push::
The name of a local ref which represents the `@{push}` location
diff --git a/ref-filter.c b/ref-filter.c
index 3bb6923..b898bcd 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -46,8 +46,10 @@ static struct used_atom {
union {
char color[COLOR_MAXLEN];
struct align align;
- enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
- remote_ref;
+ struct {
+ enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
option;
+ unsigned int nobracket: 1;
+ } remote_ref;
struct {
enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG,
C_SUB } option;
unsigned int nlines;
@@ -75,16 +77,33 @@ static void color_atom_parser(struct used_atom *atom, const
char *color_value)
static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
{
- if (!arg)
- atom->u.remote_ref = RR_NORMAL;
- else if (!strcmp(arg, "short"))
- atom->u.remote_ref = RR_SHORTEN;
- else if (!strcmp(arg, "track"))
- atom->u.remote_ref = RR_TRACK;
- else if (!strcmp(arg, "trackshort"))
- atom->u.remote_ref = RR_TRACKSHORT;
- else
- die(_("unrecognized format: %%(%s)"), atom->name);
+ struct string_list params = STRING_LIST_INIT_DUP;
+ int i;
+
+ if (!arg) {
+ atom->u.remote_ref.option = RR_NORMAL;
+ return;
+ }
+
+ atom->u.remote_ref.nobracket = 0;
+ string_list_split(¶ms, arg, ',', -1);
+
+ for (i = 0; i < params.nr; i++) {
+ const char *s = params.items[i].string;
+
+ if (!strcmp(s, "short"))
+ atom->u.remote_ref.option = RR_SHORTEN;
+ else if (!strcmp(s, "track"))
+ atom->u.remote_ref.option = RR_TRACK;
+ else if (!strcmp(s, "trackshort"))
+ atom->u.remote_ref.option = RR_TRACKSHORT;
+ else if (!strcmp(s, "nobracket"))
+ atom->u.remote_ref.nobracket = 1;
+ else
+ die(_("unrecognized format: %%(%s)"), atom->name);
+ }
+
+ string_list_clear(¶ms, 0);
}
static void body_atom_parser(struct used_atom *atom, const char *arg)
@@ -1045,25 +1064,27 @@ static void fill_remote_ref_details(struct used_atom
*atom, const char *refname,
struct branch *branch, const char **s)
{
int num_ours, num_theirs;
- if (atom->u.remote_ref == RR_SHORTEN)
+ if (atom->u.remote_ref.option == RR_SHORTEN)
*s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
- else if (atom->u.remote_ref == RR_TRACK) {
+ else if (atom->u.remote_ref.option == RR_TRACK) {
if (stat_tracking_info(branch, &num_ours,
&num_theirs, NULL)) {
- *s = "[gone]";
- return;
- }
-
- if (!num_ours && !num_theirs)
+ *s = xstrdup("gone");
+ } else if (!num_ours && !num_theirs)
*s = "";
else if (!num_ours)
- *s = xstrfmt("[behind %d]", num_theirs);
+ *s = xstrfmt("behind %d", num_theirs);
[PATCH v6 07/17] ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
Borrowing from branch.c's implementation print "[gone]" whenever an
unknown upstream ref is encountered instead of just ignoring it.
This makes sure that when branch.c is ported over to using ref-filter
APIs for printing, this feature is not lost.
Make changes to t/t6300-for-each-ref.sh and
Documentation/git-for-each-ref.txt to reflect this change.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Helped-by : Jacob Keller
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 3 ++-
ref-filter.c | 4 +++-
t/t6300-for-each-ref.sh| 2 +-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index da547c4..ba27809 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -119,7 +119,8 @@ upstream::
"[ahead N, behind M]" and `:trackshort` to show the terse
version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
or "=" (in sync). Has no effect if the ref does not have
- tracking information associated with it.
+ tracking information associated with it. `:track` also prints
+ "[gone]" whenever unknown upstream ref is encountered.
push::
The name of a local ref which represents the `@{push}` location
diff --git a/ref-filter.c b/ref-filter.c
index da2b9ee..3bb6923 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1049,8 +1049,10 @@ static void fill_remote_ref_details(struct used_atom
*atom, const char *refname,
*s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
else if (atom->u.remote_ref == RR_TRACK) {
if (stat_tracking_info(branch, &num_ours,
- &num_theirs, NULL))
+ &num_theirs, NULL)) {
+ *s = "[gone]";
return;
+ }
if (!num_ours && !num_theirs)
*s = "";
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2be0a3f..a92b36f 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -382,7 +382,7 @@ test_expect_success 'Check that :track[short] cannot be
used with other atoms' '
test_expect_success 'Check that :track[short] works when upstream is invalid' '
cat >expected <<-\EOF &&
-
+ [gone]
EOF
test_when_finished "git config branch.master.merge refs/heads/master" &&
--
2.8.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 02/17] ref-filter: include reference to 'used_atom' within 'atom_value'
Ensure that each 'atom_value' has a reference to its corresponding
'used_atom'. This let's us use values within 'used_atom' in the
'handler' function.
Hence we can get the %(align) atom's parameters directly from the
'used_atom' therefore removing the necessity of passing %(align) atom's
parameters to 'atom_value'.
This also acts as a preparatory patch for the upcoming patch where we
introduce %(if:equals=) and %(if:notequals=).
Signed-off-by: Karthik Nayak
---
ref-filter.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 41e73f0..12e646c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -230,11 +230,9 @@ struct ref_formatting_state {
struct atom_value {
const char *s;
- union {
- struct align align;
- } u;
void (*handler)(struct atom_value *atomv, struct ref_formatting_state
*state);
unsigned long ul; /* used for sorting when not FIELD_STR */
+ struct used_atom *atom;
};
/*
@@ -370,7 +368,7 @@ static void align_atom_handler(struct atom_value *atomv,
struct ref_formatting_s
push_stack_element(&state->stack);
new = state->stack;
new->at_end = end_align_handler;
- new->at_end_data = &atomv->u.align;
+ new->at_end_data = &atomv->atom->u.align;
}
static void if_then_else_handler(struct ref_formatting_stack **stack)
@@ -1069,6 +1067,7 @@ static void populate_value(struct ref_array_item *ref)
struct branch *branch = NULL;
v->handler = append_atom;
+ v->atom = atom;
if (*name == '*') {
deref = 1;
@@ -1133,7 +1132,6 @@ static void populate_value(struct ref_array_item *ref)
v->s = " ";
continue;
} else if (starts_with(name, "align")) {
- v->u.align = atom->u.align;
v->handler = align_atom_handler;
continue;
} else if (!strcmp(name, "end")) {
--
2.8.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 01/17] ref-filter: implement %(if), %(then), and %(else) atoms
Implement %(if), %(then) and %(else) atoms. Used as
%(if)...%(then)...%(end) or %(if)...%(then)...%(else)...%(end). If the
format string between %(if) and %(then) expands to an empty string, or
to only whitespaces, then the whole %(if)...%(end) expands to the string
following %(then). Otherwise, it expands to the string following
%(else), if any. Nesting of this construct is possible.
This is in preparation for porting over `git branch -l` to use
ref-filter APIs for printing.
Add Documentation and tests regarding the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 45 +++--
ref-filter.c | 133 +++--
t/t6302-for-each-ref-filter.sh | 76 +
3 files changed, 243 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index d9d406d..aea37b2 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -141,10 +141,17 @@ align::
"width=" and/or "position=" prefixes may be omitted, and bare
and used instead. For instance,
`%(align:,)`. If the contents length is more
- than the width then no alignment is performed. If used with
- '--quote' everything in between %(align:...) and %(end) is
- quoted, but if nested then only the topmost level performs
- quoting.
+ than the width then no alignment is performed.
+
+if::
+ Used as %(if)...%(then)...(%end) or
+ %(if)...%(then)...%(else)...%(end). If there is an atom with
+ value or string literal after the %(if) then everything after
+ the %(then) is printed, else if the %(else) atom is used, then
+ everything after %(else) is printed. We ignore space when
+ evaluating the string before %(then), this is useful when we
+ use the %(HEAD) atom which prints either "*" or " " and we
+ want to apply the 'if' condition only on the 'HEAD' ref.
In addition to the above, for commit and tag objects, the header
field names (`tree`, `parent`, `object`, `type`, and `tag`) can
@@ -181,6 +188,20 @@ As a special case for the date-type fields, you may
specify a format for
the date by adding `:` followed by date format name (see the
values the `--date` option to linkgit:git-rev-list[1] takes).
+Some atoms like %(align) and %(if) always require a matching %(end).
+We call them "opening atoms" and sometimes denote them as %($open).
+
+When a scripting language specific quoting is in effect (i.e. one of
+`--shell`, `--perl`, `--python`, `--tcl` is used), except for opening
+atoms, replacement from every %(atom) is quoted when and only when it
+appears at the top-level (that is, when it appears outside
+%($open)...%(end)).
+
+When a scripting language specific quoting is in effect, everything
+between a top-level opening atom and its matching %(end) is evaluated
+according to the semantics of the opening atom and its result is
+quoted.
+
EXAMPLES
@@ -268,6 +289,22 @@ eval=`git for-each-ref --shell --format="$fmt" \
eval "$eval"
+
+An example to show the usage of %(if)...%(then)...%(else)...%(end).
+This prefixes the current branch with a star.
+
+
+git for-each-ref --format="%(if)%(HEAD)%(then)* %(else)
%(end)%(refname:short)" refs/heads/
+
+
+
+An example to show the usage of %(if)...%(then)...%(end).
+This prints the authorname, if present.
+
+
+git for-each-ref --format="%(refname)%(if)%(authorname)%(then)
%(color:red)Authored by: %(authorname)%(end)"
+
+
SEE ALSO
linkgit:git-show-ref[1]
diff --git a/ref-filter.c b/ref-filter.c
index bc551a7..41e73f0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -21,6 +21,12 @@ struct align {
unsigned int width;
};
+struct if_then_else {
+ unsigned int then_atom_seen : 1,
+ else_atom_seen : 1,
+ condition_satisfied : 1;
+};
+
/*
* An atom is a valid field atom listed below, possibly prefixed with
* a "*" to denote deref_tag().
@@ -203,6 +209,9 @@ static struct {
{ "color", FIELD_STR, color_atom_parser },
{ "align", FIELD_STR, align_atom_parser },
{ "end" },
+ { "if" },
+ { "then" },
+ { "else" },
};
#define REF_FORMATTING_STATE_INIT { 0, NULL }
@@ -210,7 +219,7 @@ static struct {
struct ref_formatting_stack {
struct ref_formatting_stack *prev;
struct strbuf output;
- void (*at_end)(struct ref_formatting_stack *stack);
+ void (*at_end)(struct ref_formatting_stack **stack);
void *at_end_data;
};
@@ -343,13 +352,14 @@ static void pop_stack_element(struct ref_formatting_stack
**stack)
*stack = prev;
}
-static void end_align_handler(struct ref_formatting_stack *stack)
+static void end_align_handler(struct ref_formatting_stack **stack)
{
-
[PATCH v6 04/17] ref-filter: modify "%(objectname:short)" to take length
Add support for %(objectname:short=) which would print the
abbreviated unique objectname of given length. When no length is
specified, the length is 'DEFAULT_ABBREV'. The minimum length is
'MINIMUM_ABBREV'. The length may be exceeded to ensure that the provided
object name is unique.
Add tests and documentation for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Helped-by: Jacob Keller
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 4
ref-filter.c | 25 +++--
t/t6300-for-each-ref.sh| 10 ++
3 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 58dc9e7..da547c4 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -107,6 +107,10 @@ objectsize::
objectname::
The object name (aka SHA-1).
For a non-ambiguous abbreviation of the object name append `:short`.
+ For an abbreviation of the object name with desired length append
+ `:short=`, where the minimum length is MINIMUM_ABBREV. The
+ length may be exceeded to ensure unique object names.
+
upstream::
The name of a local ref which can be considered ``upstream''
diff --git a/ref-filter.c b/ref-filter.c
index 857a8b5..7d3af1c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -55,7 +55,10 @@ static struct used_atom {
const char *if_equals,
*not_equals;
} if_then_else;
- enum { O_FULL, O_SHORT } objectname;
+ struct {
+ enum { O_FULL, O_LENGTH, O_SHORT } option;
+ unsigned int length;
+ } objectname;
} u;
} *used_atom;
static int used_atom_cnt, need_tagged, need_symref;
@@ -118,10 +121,17 @@ static void contents_atom_parser(struct used_atom *atom,
const char *arg)
static void objectname_atom_parser(struct used_atom *atom, const char *arg)
{
if (!arg)
- atom->u.objectname = O_FULL;
+ atom->u.objectname.option = O_FULL;
else if (!strcmp(arg, "short"))
- atom->u.objectname = O_SHORT;
- else
+ atom->u.objectname.option = O_SHORT;
+ else if (skip_prefix(arg, "short=", &arg)) {
+ atom->u.objectname.option = O_LENGTH;
+ if (strtoul_ui(arg, 10, &atom->u.objectname.length) ||
+ atom->u.objectname.length == 0)
+ die(_("positive value expected objectname:short=%s"),
arg);
+ if (atom->u.objectname.length < MINIMUM_ABBREV)
+ atom->u.objectname.length = MINIMUM_ABBREV;
+ } else
die(_("unrecognized %%(objectname) argument: %s"), arg);
}
@@ -591,12 +601,15 @@ static int grab_objectname(const char *name, const
unsigned char *sha1,
struct atom_value *v, struct used_atom *atom)
{
if (starts_with(name, "objectname")) {
- if (atom->u.objectname == O_SHORT) {
+ if (atom->u.objectname.option == O_SHORT) {
v->s = xstrdup(find_unique_abbrev(sha1,
DEFAULT_ABBREV));
return 1;
- } else if (atom->u.objectname == O_FULL) {
+ } else if (atom->u.objectname.option == O_FULL) {
v->s = xstrdup(sha1_to_hex(sha1));
return 1;
+ } else if (atom->u.objectname.option == O_LENGTH) {
+ v->s = xstrdup(find_unique_abbrev(sha1,
atom->u.objectname.length));
+ return 1;
} else
die("BUG: unknown %%(objectname) option");
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 19a2823..2be0a3f 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -60,6 +60,8 @@ test_atom head objecttype commit
test_atom head objectsize 171
test_atom head objectname $(git rev-parse refs/heads/master)
test_atom head objectname:short $(git rev-parse --short refs/heads/master)
+test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
+test_atom head objectname:short=10 $(git rev-parse --short=10
refs/heads/master)
test_atom head tree $(git rev-parse refs/heads/master^{tree})
test_atom head parent ''
test_atom head numparent 0
@@ -99,6 +101,8 @@ test_atom tag objecttype tag
test_atom tag objectsize 154
test_atom tag objectname $(git rev-parse refs/tags/testtag)
test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
+test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
+test_atom head objectname:short=10 $(git rev-parse --short=10
refs/heads/master)
test_atom tag tree ''
test_atom tag parent ''
test_atom tag numparent ''
@@ -164,6 +168,12 @@ test_expect_success 'Check invalid f
[PATCH v6 09/17] ref-filter: make "%(symref)" atom work with the ':short' modifier
The "%(symref)" atom doesn't work when used with the ':short' modifier
because we strictly match only 'symref' for setting the 'need_symref'
indicator. Fix this by using comparing with valid_atom rather than used_atom.
Add tests for %(symref) and %(symref:short) while we're here.
Helped-by: Junio C Hamano
Signed-off-by: Karthik Nayak
---
ref-filter.c| 2 +-
t/t6300-for-each-ref.sh | 24
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/ref-filter.c b/ref-filter.c
index b898bcd..083cc27 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -338,7 +338,7 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
valid_atom[i].parser(&used_atom[at], arg);
if (*atom == '*')
need_tagged = 1;
- if (!strcmp(used_atom[at].name, "symref"))
+ if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
return at;
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2c5f177..b06ea1c 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -38,6 +38,7 @@ test_atom() {
case "$1" in
head) ref=refs/heads/master ;;
tag) ref=refs/tags/testtag ;;
+sym) ref=refs/heads/sym ;;
*) ref=$1 ;;
esac
printf '%s\n' "$3" >expected
@@ -565,4 +566,27 @@ test_expect_success 'Verify sort with multiple keys' '
refs/tags/bogo refs/tags/master > actual &&
test_cmp expected actual
'
+
+test_expect_success 'Add symbolic ref for the following tests' '
+ git symbolic-ref refs/heads/sym refs/heads/master
+'
+
+cat >expected < actual &&
+ test_cmp expected actual
+'
+
+cat >expected < actual &&
+ test_cmp expected actual
+'
+
test_done
--
2.8.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 15/17] branch, tag: use porcelain output
Call ref-filter's setup_ref_filter_porcelain_msg() to enable translated messages for the %(upstream:tack) atom. Although branch.c doesn't currently use ref-filter's printing API's, this will ensure that when it does in the future patches, we do not need to worry about translation. Written-by: Matthieu Moy Mentored-by: Christian Couder Mentored-by: Matthieu Moy Signed-off-by: Karthik Nayak --- builtin/branch.c | 2 ++ builtin/tag.c| 2 ++ 2 files changed, 4 insertions(+) diff --git a/builtin/branch.c b/builtin/branch.c index 141168d..fc5eae0 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -656,6 +656,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix) OPT_END(), }; + setup_ref_filter_porcelain_msg(); + memset(&filter, 0, sizeof(filter)); filter.kind = FILTER_REFS_BRANCHES; filter.abbrev = -1; diff --git a/builtin/tag.c b/builtin/tag.c index 50e4ae5..a00e9a7 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -373,6 +373,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix) OPT_END() }; + setup_ref_filter_porcelain_msg(); + git_config(git_tag_config, sorting_tail); memset(&opt, 0, sizeof(opt)); -- 2.8.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 16/17] branch: use ref-filter printing APIs
Port branch.c to use ref-filter APIs for printing. This clears out
most of the code used in branch.c for printing and replaces them with
calls made to the ref-filter library.
Introduce build_format() which gets the format required for printing
of refs. Make amendments to print_ref_list() to reflect these changes.
Change calc_maxwidth() to also account for the length of HEAD ref, by
calling ref-filter:get_head_discription().
Also change the test in t6040 to reflect the changes.
Before this patch, all cross-prefix symrefs weren't shortened. Since
we're using ref-filter APIs, we shorten all symrefs by default. We also
allow the user to change the format if needed with the introduction of
the '--format' option in the next patch.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/branch.c | 234 ++-
t/t3203-branch-output.sh | 2 +-
t/t6040-tracking-info.sh | 2 +-
3 files changed, 70 insertions(+), 168 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index fc5eae0..125085f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -36,12 +36,12 @@ static unsigned char head_sha1[20];
static int branch_use_color = -1;
static char branch_colors[][COLOR_MAXLEN] = {
- GIT_COLOR_RESET,
- GIT_COLOR_NORMAL, /* PLAIN */
- GIT_COLOR_RED, /* REMOTE */
- GIT_COLOR_NORMAL, /* LOCAL */
- GIT_COLOR_GREEN,/* CURRENT */
- GIT_COLOR_BLUE, /* UPSTREAM */
+ "%(color:reset)",
+ "%(color:reset)", /* PLAIN */
+ "%(color:red)", /* REMOTE */
+ "%(color:reset)", /* LOCAL */
+ "%(color:green)", /* CURRENT */
+ "%(color:blue)",/* UPSTREAM */
};
enum color_branch {
BRANCH_COLOR_RESET = 0,
@@ -277,162 +277,6 @@ static int delete_branches(int argc, const char **argv,
int force, int kinds,
return(ret);
}
-static void fill_tracking_info(struct strbuf *stat, const char *branch_name,
- int show_upstream_ref)
-{
- int ours, theirs;
- char *ref = NULL;
- struct branch *branch = branch_get(branch_name);
- const char *upstream;
- struct strbuf fancy = STRBUF_INIT;
- int upstream_is_gone = 0;
- int added_decoration = 1;
-
- if (stat_tracking_info(branch, &ours, &theirs, &upstream) < 0) {
- if (!upstream)
- return;
- upstream_is_gone = 1;
- }
-
- if (show_upstream_ref) {
- ref = shorten_unambiguous_ref(upstream, 0);
- if (want_color(branch_use_color))
- strbuf_addf(&fancy, "%s%s%s",
- branch_get_color(BRANCH_COLOR_UPSTREAM),
- ref,
branch_get_color(BRANCH_COLOR_RESET));
- else
- strbuf_addstr(&fancy, ref);
- }
-
- if (upstream_is_gone) {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s: gone]"), fancy.buf);
- else
- added_decoration = 0;
- } else if (!ours && !theirs) {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s]"), fancy.buf);
- else
- added_decoration = 0;
- } else if (!ours) {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s: behind %d]"), fancy.buf,
theirs);
- else
- strbuf_addf(stat, _("[behind %d]"), theirs);
-
- } else if (!theirs) {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s: ahead %d]"), fancy.buf, ours);
- else
- strbuf_addf(stat, _("[ahead %d]"), ours);
- } else {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s: ahead %d, behind %d]"),
- fancy.buf, ours, theirs);
- else
- strbuf_addf(stat, _("[ahead %d, behind %d]"),
- ours, theirs);
- }
- strbuf_release(&fancy);
- if (added_decoration)
- strbuf_addch(stat, ' ');
- free(ref);
-}
-
-static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,
-struct ref_filter *filter, const char *refname)
-{
- struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
- const char *sub = _(" invalid ref ");
- struct commit *commit = item->commit;
-
- if (!parse_commit(commit)) {
- pp_commit_easy(CMIT_FMT_ONELINE, commit, &subject);
- sub = subject.buf;
- }
-
- if (item->kind == FILTER_REFS_BRANCHES)
- fill_tracking_info(&stat, refname, filter->verbose > 1);
-
- strbuf_addf(out, " %s %s
[PATCH v6 13/17] ref-filter: add `:dir` and `:base` options for ref printing atoms
Add the options `:dir` and `:base` to all ref printing ('%(refname)',
'%(symref)', '%(push)' and '%(upstream)') atoms. The `:dir` option gives
the directory (the part after $GIT_DIR/) of the ref without the
refname. The `:base` option gives the base directory of the given
ref (i.e. the directory following $GIT_DIR/refs/).
Add tests and documentation for the same.
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 34 +++---
ref-filter.c | 29 +
t/t6300-for-each-ref.sh| 24
3 files changed, 68 insertions(+), 19 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index b472ee9..cda1af1 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -96,7 +96,9 @@ refname::
slash-separated path components from the front of the refname
(e.g., `%(refname:strip=2)` turns `refs/tags/foo` into `foo`.
`` must be a positive integer. If a displayed ref has fewer
- components than ``, the command aborts with an error.
+ components than ``, the command aborts with an error. For the base
+ directory of the ref (i.e. foo in refs/foo/bar/boz) append
+ `:base`. For the entire directory path append `:dir`.
objecttype::
The type of the object (`blob`, `tree`, `commit`, `tag`).
@@ -114,22 +116,23 @@ objectname::
upstream::
The name of a local ref which can be considered ``upstream''
- from the displayed ref. Respects `:short` and `:strip` in the
- same way as `refname` above. Additionally respects `:track`
- to show "[ahead N, behind M]" and `:trackshort` to show the
- terse version: ">" (ahead), "<" (behind), "<>" (ahead and
- behind), or "=" (in sync). `:track` also prints "[gone]"
- whenever unknown upstream ref is encountered. Append
- `:track,nobracket` to show tracking information without
- brackets (i.e "ahead N, behind M"). Has no effect if the ref
- does not have tracking information associated with it.
+ from the displayed ref. Respects `:short`, `:strip`, `:base`
+ and `:dir` in the same way as `refname` above. Additionally
+ respects `:track` to show "[ahead N, behind M]" and
+ `:trackshort` to show the terse version: ">" (ahead), "<"
+ (behind), "<>" (ahead and behind), or "=" (in sync). `:track`
+ also prints "[gone]" whenever unknown upstream ref is
+ encountered. Append `:track,nobracket` to show tracking
+ information without brackets (i.e "ahead N, behind M"). Has
+ no effect if the ref does not have tracking information
+ associated with it.
push::
The name of a local ref which represents the `@{push}`
location for the displayed ref. Respects `:short`, `:strip`,
- `:track`, and `:trackshort` options as `upstream`
- does. Produces an empty string if no `@{push}` ref is
- configured.
+ `:track`, `:trackshort`, `:base` and `:dir` options as
+ `upstream` does. Produces an empty string if no `@{push}` ref
+ is configured.
HEAD::
'*' if HEAD matches current ref (the checked out branch), ' '
@@ -166,8 +169,9 @@ if::
symref::
The ref which the given symbolic ref refers to. If not a
- symbolic ref, nothing is printed. Respects the `:short` and
- `:strip` options in the same way as `refname` above.
+ symbolic ref, nothing is printed. Respects the `:short`,
+ `:strip`, `:base` and `:dir` options in the same way as
+ `refname` above.
In addition to the above, for commit and tag objects, the header
field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/ref-filter.c b/ref-filter.c
index 3b42aab..97977a5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -31,7 +31,7 @@ struct if_then_else {
};
struct refname_atom {
- enum { R_NORMAL, R_SHORT, R_STRIP } option;
+ enum { R_BASE, R_DIR, R_NORMAL, R_SHORT, R_STRIP } option;
unsigned int strip;
};
@@ -93,7 +93,11 @@ static void refname_atom_parser_internal(struct refname_atom
*atom,
atom->option = R_STRIP;
if (strtoul_ui(arg, 10, &atom->strip) || atom->strip <= 0)
die(_("positive value expected refname:strip=%s"), arg);
- } else
+ } else if (!strcmp(arg, "dir"))
+ atom->option = R_DIR;
+ else if (!strcmp(arg, "base"))
+ atom->option = R_BASE;
+ else
die(_("unrecognized %%(%s) argument: %s"), name, arg);
}
@@ -252,7 +256,6 @@ static void if_atom_parser(struct used_atom *atom, const
char *arg)
die(_("unrecognized %%(if) argument: %s"), arg);
}
-
static struct {
const char *name;
cmp_type cmp_type;
@@ -1096,7 +1099,25 @@ static const char *show_ref(struct refname
[PATCH v6 11/17] ref-filter: introduce symref_atom_parser() and refname_atom_parser()
Using refname_atom_parser_internal(), introduce symref_atom_parser() and
refname_atom_parser() which will parse the atoms %(symref) and
%(refname) respectively. Store the parsed information into the
'used_atom' structure based on the modifiers used along with the atoms.
Now the '%(symref)' atom supports the ':strip' atom modifier. Update the
Documentation and tests to reflect this.
Helped-by: Jeff King
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 5 +++
ref-filter.c | 78 ++
t/t6300-for-each-ref.sh| 9 +
3 files changed, 59 insertions(+), 33 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 7f0cb19..6ab84b8 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -163,6 +163,11 @@ if::
the value between the %(if:...) and %(then) atoms with the
given string.
+symref::
+ The ref which the given symbolic ref refers to. If not a
+ symbolic ref, nothing is printed. Respects the `:short` and
+ `:strip` options in the same way as `refname` above.
+
In addition to the above, for commit and tag objects, the header
field names (`tree`, `parent`, `object`, `type`, and `tag`) can
be used to specify the value in the header field.
diff --git a/ref-filter.c b/ref-filter.c
index 778fe02..933f8ca 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -176,6 +176,16 @@ static void objectname_atom_parser(struct used_atom *atom,
const char *arg)
die(_("unrecognized %%(objectname) argument: %s"), arg);
}
+static void symref_atom_parser(struct used_atom *atom, const char *arg)
+{
+ return refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
+}
+
+static void refname_atom_parser(struct used_atom *atom, const char *arg)
+{
+ return refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
+}
+
static align_type parse_align_position(const char *s)
{
if (!strcmp(s, "right"))
@@ -244,7 +254,7 @@ static struct {
cmp_type cmp_type;
void (*parser)(struct used_atom *atom, const char *arg);
} valid_atom[] = {
- { "refname" },
+ { "refname" , FIELD_STR, refname_atom_parser },
{ "objecttype" },
{ "objectsize", FIELD_ULONG },
{ "objectname", FIELD_STR, objectname_atom_parser },
@@ -273,7 +283,7 @@ static struct {
{ "contents", FIELD_STR, contents_atom_parser },
{ "upstream", FIELD_STR, remote_ref_atom_parser },
{ "push", FIELD_STR, remote_ref_atom_parser },
- { "symref" },
+ { "symref", FIELD_STR, symref_atom_parser },
{ "flag" },
{ "HEAD" },
{ "color", FIELD_STR, color_atom_parser },
@@ -1058,21 +1068,16 @@ static inline char *copy_advance(char *dst, const char
*src)
return dst;
}
-static const char *strip_ref_components(const char *refname, const char
*nr_arg)
+static const char *strip_ref_components(const char *refname, unsigned int len)
{
- char *end;
- long nr = strtol(nr_arg, &end, 10);
- long remaining = nr;
+ long remaining = len;
const char *start = refname;
- if (nr < 1 || *end != '\0')
- die(_(":strip= requires a positive integer argument"));
-
while (remaining) {
switch (*start++) {
case '\0':
- die(_("ref '%s' does not have %ld components to
:strip"),
- refname, nr);
+ die(_("ref '%s' does not have %ud components to
:strip"),
+ refname, len);
case '/':
remaining--;
break;
@@ -1081,6 +1086,16 @@ static const char *strip_ref_components(const char
*refname, const char *nr_arg)
return start;
}
+static const char *show_ref(struct refname_atom *atom, const char *refname)
+{
+ if (atom->option == R_SHORT)
+ return shorten_unambiguous_ref(refname, warn_ambiguous_refs);
+ else if (atom->option == R_STRIP)
+ return strip_ref_components(refname, atom->strip);
+ else
+ return refname;
+}
+
static void fill_remote_ref_details(struct used_atom *atom, const char
*refname,
struct branch *branch, const char **s)
{
@@ -1153,6 +1168,21 @@ char *get_head_description(void)
return strbuf_detach(&desc, NULL);
}
+static const char *get_symref(struct used_atom *atom, struct ref_array_item
*ref)
+{
+ if (!ref->symref)
+ return "";
+ else
+ return show_ref(&atom->u.refname, ref->symref);
+}
+
+static const char *get_refname(struct used_atom *atom, struct ref_array_item
*ref)
+{
+ if (ref->kind & FILTER_REFS_DETACHED_HEAD)
+ return get_head_description();
+ return show_ref(&atom->u.refname, ref->refname);
+}
[PATCH v6 10/17] ref-filter: introduce refname_atom_parser_internal()
Since there are multiple atoms which print refs ('%(refname)',
'%(symref)', '%(push)', '%upstream'), it makes sense to have a common
ground for parsing them. This would allow us to share implementations of
the atom modifiers between these atoms.
Introduce refname_atom_parser_internal() to act as a common parsing
function for ref printing atoms. This would eventually be used to
introduce refname_atom_parser() and symref_atom_parser() and also be
internally used in remote_ref_atom_parser().
Helped-by: Jeff King
Signed-off-by: Karthik Nayak
---
ref-filter.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/ref-filter.c b/ref-filter.c
index 083cc27..778fe02 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -30,6 +30,11 @@ struct if_then_else {
condition_satisfied : 1;
};
+struct refname_atom {
+ enum { R_NORMAL, R_SHORT, R_STRIP } option;
+ unsigned int strip;
+};
+
/*
* An atom is a valid field atom listed below, possibly prefixed with
* a "*" to denote deref_tag().
@@ -62,6 +67,7 @@ static struct used_atom {
enum { O_FULL, O_LENGTH, O_SHORT } option;
unsigned int length;
} objectname;
+ struct refname_atom refname;
} u;
} *used_atom;
static int used_atom_cnt, need_tagged, need_symref;
@@ -75,6 +81,21 @@ static void color_atom_parser(struct used_atom *atom, const
char *color_value)
die(_("unrecognized color: %%(color:%s)"), color_value);
}
+static void refname_atom_parser_internal(struct refname_atom *atom,
+const char *arg, const char *name)
+{
+ if (!arg)
+ atom->option = R_NORMAL;
+ else if (!strcmp(arg, "short"))
+ atom->option = R_SHORT;
+ else if (skip_prefix(arg, "strip=", &arg)) {
+ atom->option = R_STRIP;
+ if (strtoul_ui(arg, 10, &atom->strip) || atom->strip <= 0)
+ die(_("positive value expected refname:strip=%s"), arg);
+ } else
+ die(_("unrecognized %%(%s) argument: %s"), name, arg);
+}
+
static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
{
struct string_list params = STRING_LIST_INIT_DUP;
--
2.8.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 14/17] ref-filter: allow porcelain to translate messages in the output
Introduce setup_ref_filter_porcelain_msg() so that the messages used in
the atom %(upstream:track) can be translated if needed. This is needed
as we port branch.c to use ref-filter's printing API's.
Written-by: Matthieu Moy
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
ref-filter.c | 28
ref-filter.h | 2 ++
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 97977a5..74c4869 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -15,6 +15,26 @@
#include "version.h"
#include "wt-status.h"
+static struct ref_msg {
+ const char *gone;
+ const char *ahead;
+ const char *behind;
+ const char *ahead_behind;
+} msgs = {
+ "gone",
+ "ahead %d",
+ "behind %d",
+ "ahead %d, behind %d"
+};
+
+void setup_ref_filter_porcelain_msg(void)
+{
+ msgs.gone = _("gone");
+ msgs.ahead = _("ahead %d");
+ msgs.behind = _("behind %d");
+ msgs.ahead_behind = _("ahead %d, behind %d");
+}
+
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
struct align {
@@ -1130,15 +1150,15 @@ static void fill_remote_ref_details(struct used_atom
*atom, const char *refname,
else if (atom->u.remote_ref.option == RR_TRACK) {
if (stat_tracking_info(branch, &num_ours,
&num_theirs, NULL)) {
- *s = xstrdup("gone");
+ *s = xstrdup(msgs.gone);
} else if (!num_ours && !num_theirs)
*s = "";
else if (!num_ours)
- *s = xstrfmt("behind %d", num_theirs);
+ *s = xstrfmt(msgs.behind, num_theirs);
else if (!num_theirs)
- *s = xstrfmt("ahead %d", num_ours);
+ *s = xstrfmt(msgs.ahead, num_ours);
else
- *s = xstrfmt("ahead %d, behind %d",
+ *s = xstrfmt(msgs.ahead_behind,
num_ours, num_theirs);
if (!atom->u.remote_ref.nobracket && *s[0]) {
const char *to_free = *s;
diff --git a/ref-filter.h b/ref-filter.h
index 0014b92..da17145 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -111,5 +111,7 @@ struct ref_sorting *ref_default_sorting(void);
int parse_opt_merge_filter(const struct option *opt, const char *arg, int
unset);
/* Get the current HEAD's description */
char *get_head_description(void);
+/* Set up translated strings in the output. */
+void setup_ref_filter_porcelain_msg(void);
#endif /* REF_FILTER_H */
--
2.8.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 12/17] ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
Use the recently introduced refname_atom_parser_internal() within
remote_ref_atom_parser(), this provides a common base for all the ref
printing atoms, allowing %(upstream) and %(push) to also use the
':strip' option.
The atoms '%(push)' and '%(upstream)' will retain the ':track' and
':trackshort' atom modifiers to themselves as they have no meaning in
context to the '%(refname)' and '%(symref)' atoms.
Update the documentation and tests to reflect the same.
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 27 ++-
ref-filter.c | 26 +++---
t/t6300-for-each-ref.sh| 2 ++
3 files changed, 31 insertions(+), 24 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 6ab84b8..b472ee9 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -114,21 +114,22 @@ objectname::
upstream::
The name of a local ref which can be considered ``upstream''
- from the displayed ref. Respects `:short` in the same way as
- `refname` above. Additionally respects `:track` to show
- "[ahead N, behind M]" and `:trackshort` to show the terse
- version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
- or "=" (in sync). `:track` also prints "[gone]" whenever
- unknown upstream ref is encountered. Append `:track,nobracket`
- to show tracking information without brackets (i.e "ahead N,
- behind M"). Has no effect if the ref does not have tracking
- information associated with it.
+ from the displayed ref. Respects `:short` and `:strip` in the
+ same way as `refname` above. Additionally respects `:track`
+ to show "[ahead N, behind M]" and `:trackshort` to show the
+ terse version: ">" (ahead), "<" (behind), "<>" (ahead and
+ behind), or "=" (in sync). `:track` also prints "[gone]"
+ whenever unknown upstream ref is encountered. Append
+ `:track,nobracket` to show tracking information without
+ brackets (i.e "ahead N, behind M"). Has no effect if the ref
+ does not have tracking information associated with it.
push::
- The name of a local ref which represents the `@{push}` location
- for the displayed ref. Respects `:short`, `:track`, and
- `:trackshort` options as `upstream` does. Produces an empty
- string if no `@{push}` ref is configured.
+ The name of a local ref which represents the `@{push}`
+ location for the displayed ref. Respects `:short`, `:strip`,
+ `:track`, and `:trackshort` options as `upstream`
+ does. Produces an empty string if no `@{push}` ref is
+ configured.
HEAD::
'*' if HEAD matches current ref (the checked out branch), ' '
diff --git a/ref-filter.c b/ref-filter.c
index 933f8ca..3b42aab 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -52,7 +52,8 @@ static struct used_atom {
char color[COLOR_MAXLEN];
struct align align;
struct {
- enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
option;
+ enum { RR_REF, RR_TRACK, RR_TRACKSHORT } option;
+ struct refname_atom refname;
unsigned int nobracket: 1;
} remote_ref;
struct {
@@ -102,7 +103,9 @@ static void remote_ref_atom_parser(struct used_atom *atom,
const char *arg)
int i;
if (!arg) {
- atom->u.remote_ref.option = RR_NORMAL;
+ atom->u.remote_ref.option = RR_REF;
+ refname_atom_parser_internal(&atom->u.remote_ref.refname,
+arg, atom->name);
return;
}
@@ -112,16 +115,17 @@ static void remote_ref_atom_parser(struct used_atom
*atom, const char *arg)
for (i = 0; i < params.nr; i++) {
const char *s = params.items[i].string;
- if (!strcmp(s, "short"))
- atom->u.remote_ref.option = RR_SHORTEN;
- else if (!strcmp(s, "track"))
+ if (!strcmp(s, "track"))
atom->u.remote_ref.option = RR_TRACK;
else if (!strcmp(s, "trackshort"))
atom->u.remote_ref.option = RR_TRACKSHORT;
else if (!strcmp(s, "nobracket"))
atom->u.remote_ref.nobracket = 1;
- else
- die(_("unrecognized format: %%(%s)"), atom->name);
+ else {
+ atom->u.remote_ref.option = RR_REF;
+
refname_atom_parser_internal(&atom->u.remote_ref.refname,
+arg, atom->name);
+ }
}
string_list_clear(¶ms, 0);
@@ -1100,8 +1104,8 @@ static void fill_remote_ref_details(struct used_atom
*atom, cons
[PATCH v6 17/17] branch: implement '--format' option
Implement the '--format' option provided by 'ref-filter'. This lets the
user list branches as per desired format similar to the implementation
in 'git for-each-ref'.
Add tests and documentation for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-branch.txt | 7 ++-
builtin/branch.c | 14 +-
t/t3203-branch-output.sh | 14 ++
3 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 4a7037f..8af132f 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[--list] [-v [--abbrev= | --no-abbrev]]
[--column[=] | --no-column]
[(--merged | --no-merged | --contains) []] [--sort=]
- [--points-at ] [...]
+ [--points-at ] [--format=] [...]
'git branch' [--set-upstream | --track | --no-track] [-l] [-f]
[]
'git branch' (--set-upstream-to= | -u ) []
'git branch' --unset-upstream []
@@ -246,6 +246,11 @@ start-point is either a local or remote-tracking branch.
--points-at ::
Only list branches of the given object.
+--format ::
+ A string that interpolates `%(fieldname)` from the object
+ pointed at by a ref being shown. The format is the same as
+ that of linkgit:git-for-each-ref[1].
+
Examples
diff --git a/builtin/branch.c b/builtin/branch.c
index 125085f..3b0f655 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -28,6 +28,7 @@ static const char * const builtin_branch_usage[] = {
N_("git branch [] [-r] (-d | -D) ..."),
N_("git branch [] (-m | -M) [] "),
N_("git branch [] [-r | -a] [--points-at]"),
+ N_("git branch [] [-r | -a] [--format]"),
NULL
};
@@ -340,14 +341,14 @@ static char *build_format(struct ref_filter *filter, int
maxwidth, const char *r
return strbuf_detach(&fmt, NULL);
}
-static void print_ref_list(struct ref_filter *filter, struct ref_sorting
*sorting)
+static void print_ref_list(struct ref_filter *filter, struct ref_sorting
*sorting, const char *format)
{
int i;
struct ref_array array;
int maxwidth = 0;
const char *remote_prefix = "";
struct strbuf out = STRBUF_INIT;
- char *format;
+ char *to_free = NULL;
/*
* If we are listing more than just remote branches,
@@ -364,7 +365,8 @@ static void print_ref_list(struct ref_filter *filter,
struct ref_sorting *sortin
if (filter->verbose)
maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
- format = build_format(filter, maxwidth, remote_prefix);
+ if (!format)
+ format = to_free = build_format(filter, maxwidth,
remote_prefix);
verify_ref_format(format);
/*
@@ -392,7 +394,7 @@ static void print_ref_list(struct ref_filter *filter,
struct ref_sorting *sortin
}
ref_array_clear(&array);
- free(format);
+ free(to_free);
}
static void reject_rebase_or_bisect_branch(const char *target)
@@ -515,6 +517,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
enum branch_track track;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
+ const char *format = NULL;
struct option options[] = {
OPT_GROUP(N_("Generic options")),
@@ -555,6 +558,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
OPTION_CALLBACK, 0, "points-at", &filter.points_at,
N_("object"),
N_("print only branches of the object"), 0,
parse_opt_object_name
},
+ OPT_STRING( 0 , "format", &format, N_("format"), N_("format to
use for the output")),
OPT_END(),
};
@@ -615,7 +619,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
filter.kind |= FILTER_REFS_DETACHED_HEAD;
filter.name_patterns = argv;
- print_ref_list(&filter, sorting);
+ print_ref_list(&filter, sorting, format);
print_columns(&output, colopts, NULL);
string_list_clear(&output, 0);
return 0;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 980c732..d8edaf2 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -196,4 +196,18 @@ test_expect_success 'local-branch symrefs shortened
properly' '
test_cmp expect actual
'
+test_expect_success 'git branch --format option' '
+ cat >expect <<-\EOF &&
+ Refname is (HEAD detached from fromtag)
+ Refname is refs/heads/ambiguous
+ Refname is refs/heads/branch-one
+ Refname is refs/heads/branch-two
+ Refname is refs/heads/master
+
[PATCH v2 0/3] CRLF-Handling: bug fix around ce_compare_data()
From: Torsten Bögershausen Changes since v1: - Re-order the patches - t6038; use crlf on all platforms This did actually not break anything (without old 7/10) Adapt the commit message - ce_compare_data(): Simplify the logic around free() in has_cr_in_index(), Thanks Eric Sunshine Break up the old 10/10 series about CLRF handling into smaller series. 1/10..4/10 are now found in tb/core-eol-fix This series includes 3 from the 10/10 series: 05/10 read-cache: factor out get_sha1_from_index() helper #now 1/3 10/10 Fix for ce_compare_data(), done right. #now 2/3 09/10 t6038; use crlf on all platforms#now 3/3 The most important patch is 2/3 The reminding 3 patches, stream-and-early-out, convert-unify-the-auto-handling-of-CRLF more-safer-crlf-handling-with-text-attr Need to be re-done, re-sorted and re-written, thanks for all the comments. Torsten Bögershausen (3): read-cache: factor out get_sha1_from_index() helper convert: ce_compare_data() checks for a sha1 of a path t6038; use crlf on all platforms cache.h| 4 convert.c | 34 ++ convert.h | 23 +++ read-cache.c | 33 + sha1_file.c| 17 + t/t6038-merge-text-auto.sh | 37 +++-- 6 files changed, 90 insertions(+), 58 deletions(-) -- 2.0.0.rc1.6318.g0c2c796 -- 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/3] t6038; use crlf on all platforms
From: Torsten Bögershausen t6038 uses different code, dependig if NATIVE_CRLF is set ot not. When the native line endings are LF, merge.renormalize is not tested very well. Change the test to always use CRLF by setting core.eol=crlf. --- t/t6038-merge-text-auto.sh | 37 +++-- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh index 85c10b0..4dc8c1a 100755 --- a/t/t6038-merge-text-auto.sh +++ b/t/t6038-merge-text-auto.sh @@ -18,6 +18,7 @@ test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b test_expect_success setup ' git config core.autocrlf false && + git config core.eol crlf && echo first line | append_cr >file && echo first line >control_file && @@ -72,10 +73,8 @@ test_expect_success 'Merge after setting text=auto' ' same line EOF - if test_have_prereq NATIVE_CRLF; then - append_cr expected.temp && - mv expected.temp expected - fi && + append_cr expected.temp && + mv expected.temp expected && git config merge.renormalize true && git rm -fr . && rm -f .gitattributes && @@ -90,10 +89,8 @@ test_expect_success 'Merge addition of text=auto' ' same line EOF - if test_have_prereq NATIVE_CRLF; then - append_cr expected.temp && - mv expected.temp expected - fi && + append_cr expected.temp && + mv expected.temp expected && git config merge.renormalize true && git rm -fr . && rm -f .gitattributes && @@ -104,15 +101,9 @@ test_expect_success 'Merge addition of text=auto' ' test_expect_success 'Detect CRLF/LF conflict after setting text=auto' ' echo "<<<" >expected && - if test_have_prereq NATIVE_CRLF; then - echo first line | append_cr >>expected && - echo same line | append_cr >>expected && - echo === | append_cr >>expected - else - echo first line >>expected && - echo same line >>expected && - echo === >>expected - fi && + echo first line | append_cr >>expected && + echo same line | append_cr >>expected && + echo === | append_cr >>expected && echo first line | append_cr >>expected && echo same line | append_cr >>expected && echo ">>>" >>expected && @@ -128,15 +119,9 @@ test_expect_success 'Detect LF/CRLF conflict from addition of text=auto' ' echo "<<<" >expected && echo first line | append_cr >>expected && echo same line | append_cr >>expected && - if test_have_prereq NATIVE_CRLF; then - echo === | append_cr >>expected && - echo first line | append_cr >>expected && - echo same line | append_cr >>expected - else - echo === >>expected && - echo first line >>expected && - echo same line >>expected - fi && + echo === | append_cr >>expected && + echo first line | append_cr >>expected && + echo same line | append_cr >>expected && echo ">>>" >>expected && git config merge.renormalize false && rm -f .gitattributes && -- 2.0.0.rc1.6318.g0c2c796 -- 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/3] read-cache: factor out get_sha1_from_index() helper
From: Torsten Bögershausen
Factor out the retrieval of the sha1 for a given path in
read_blob_data_from_index() into the function get_sha1_from_index().
This will be used in the next commit, when convert.c can do the
analyze for "text=auto" without slurping the whole blob into memory
at once.
Add a wrapper definition get_sha1_from_cache().
Signed-off-by: Torsten Bögershausen
---
cache.h | 3 +++
read-cache.c | 29 ++---
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/cache.h b/cache.h
index 160f8e3..15a2a10 100644
--- a/cache.h
+++ b/cache.h
@@ -379,6 +379,7 @@ extern void free_name_hash(struct index_state *istate);
#define unmerge_cache_entry_at(at) unmerge_index_entry_at(&the_index, at)
#define unmerge_cache(pathspec) unmerge_index(&the_index, pathspec)
#define read_blob_data_from_cache(path, sz)
read_blob_data_from_index(&the_index, (path), (sz))
+#define get_sha1_from_cache(path) get_sha1_from_index (&the_index, (path))
#endif
enum object_type {
@@ -1038,6 +1039,8 @@ static inline void *read_sha1_file(const unsigned char
*sha1, enum object_type *
return read_sha1_file_extended(sha1, type, size, LOOKUP_REPLACE_OBJECT);
}
+const unsigned char *get_sha1_from_index(struct index_state *istate, const
char *path);
+
/*
* This internal function is only declared here for the benefit of
* lookup_replace_object(). Please do not call it directly.
diff --git a/read-cache.c b/read-cache.c
index d9fb78b..a3ef967 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2263,13 +2263,27 @@ int index_name_is_other(const struct index_state
*istate, const char *name,
void *read_blob_data_from_index(struct index_state *istate, const char *path,
unsigned long *size)
{
- int pos, len;
+ const unsigned char *sha1;
unsigned long sz;
enum object_type type;
void *data;
- len = strlen(path);
- pos = index_name_pos(istate, path, len);
+ sha1 = get_sha1_from_index(istate, path);
+ if (!sha1)
+ return NULL;
+ data = read_sha1_file(sha1, &type, &sz);
+ if (!data || type != OBJ_BLOB) {
+ free(data);
+ return NULL;
+ }
+ if (size)
+ *size = sz;
+ return data;
+}
+
+const unsigned char *get_sha1_from_index(struct index_state *istate, const
char *path)
+{
+ int pos = index_name_pos(istate, path, strlen(path));
if (pos < 0) {
/*
* We might be in the middle of a merge, in which
@@ -2285,14 +2299,7 @@ void *read_blob_data_from_index(struct index_state
*istate, const char *path, un
}
if (pos < 0)
return NULL;
- data = read_sha1_file(istate->cache[pos]->sha1, &type, &sz);
- if (!data || type != OBJ_BLOB) {
- free(data);
- return NULL;
- }
- if (size)
- *size = sz;
- return data;
+ return (istate->cache[pos]->sha1);
}
void stat_validity_clear(struct stat_validity *sv)
--
2.0.0.rc1.6318.g0c2c796
--
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/3] convert: ce_compare_data() checks for a sha1 of a path
From: Torsten Bögershausen
To compare a file in working tree with the index, convert_to_git() is used,
the the result is hashed and the hash value compared with ce->sha1.
Deep down would_convert_crlf_at_commit() is invoked, to check if CRLF
are converted or not: When a CRLF had been in the index before, CRLF in
the working tree are not converted.
While in a merge, a file name in the working tree has different blobs
in the index with different hash values.
Forwarding ce->sha1 from ce_compare_data() into crlf_to_git() makes sure
the would_convert_crlf_at_commit() looks at the appropriate blob.
Signed-off-by: Torsten Bögershausen
---
cache.h | 1 +
convert.c| 34 ++
convert.h| 23 +++
read-cache.c | 4 +++-
sha1_file.c | 17 +
5 files changed, 58 insertions(+), 21 deletions(-)
diff --git a/cache.h b/cache.h
index 15a2a10..a5136c0 100644
--- a/cache.h
+++ b/cache.h
@@ -605,6 +605,7 @@ extern int ie_modified(const struct index_state *, const
struct cache_entry *, s
#define HASH_WRITE_OBJECT 1
#define HASH_FORMAT_CHECK 2
+#define HASH_CE_HAS_SHA1 4
extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum
object_type type, const char *path, unsigned flags);
extern int index_path(unsigned char *sha1, const char *path, struct stat *st,
unsigned flags);
diff --git a/convert.c b/convert.c
index f524b8d..c972d14 100644
--- a/convert.c
+++ b/convert.c
@@ -217,21 +217,28 @@ static void check_safe_crlf(const char *path, enum
crlf_action crlf_action,
}
}
-static int has_cr_in_index(const char *path)
+static int has_cr_in_index(const char *path, const unsigned char *sha1)
{
unsigned long sz;
void *data;
- int has_cr;
-
- data = read_blob_data_from_cache(path, &sz);
+ int has_cr = 0;
+ enum object_type type;
+ if (!sha1)
+ sha1 = get_sha1_from_cache(path);
+ if (!sha1)
+ return 0;
+ data = read_sha1_file(sha1, &type, &sz);
if (!data)
return 0;
- has_cr = memchr(data, '\r', sz) != NULL;
+ if (type == OBJ_BLOB)
+ has_cr = memchr(data, '\r', sz) != NULL;
+
free(data);
return has_cr;
}
-static int crlf_to_git(const char *path, const char *src, size_t len,
+static int crlf_to_git(const char *path, const unsigned char *sha1,
+ const char *src, size_t len,
struct strbuf *buf,
enum crlf_action crlf_action, enum safe_crlf checksafe)
{
@@ -260,7 +267,7 @@ static int crlf_to_git(const char *path, const char *src,
size_t len,
* If the file in the index has any CR in it, do not
convert.
* This is the new safer autocrlf handling.
*/
- if (has_cr_in_index(path))
+ if (has_cr_in_index(path, sha1))
return 0;
}
}
@@ -852,8 +859,9 @@ const char *get_convert_attr_ascii(const char *path)
return "";
}
-int convert_to_git(const char *path, const char *src, size_t len,
- struct strbuf *dst, enum safe_crlf checksafe)
+int convert_to_git_ce_sha1(const char *path, const unsigned char *sha1,
+ const char *src, size_t len,
+ struct strbuf *dst, enum safe_crlf checksafe)
{
int ret = 0;
const char *filter = NULL;
@@ -874,7 +882,7 @@ int convert_to_git(const char *path, const char *src,
size_t len,
src = dst->buf;
len = dst->len;
}
- ret |= crlf_to_git(path, src, len, dst, ca.crlf_action, checksafe);
+ ret |= crlf_to_git(path, sha1, src, len, dst, ca.crlf_action,
checksafe);
if (ret && dst) {
src = dst->buf;
len = dst->len;
@@ -882,7 +890,9 @@ int convert_to_git(const char *path, const char *src,
size_t len,
return ret | ident_to_git(path, src, len, dst, ca.ident);
}
-void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,
+void convert_to_git_filter_fd(const char *path,
+ const unsigned char *sha1,
+ int fd, struct strbuf *dst,
enum safe_crlf checksafe)
{
struct conv_attrs ca;
@@ -894,7 +904,7 @@ void convert_to_git_filter_fd(const char *path, int fd,
struct strbuf *dst,
if (!apply_filter(path, NULL, 0, fd, dst, ca.drv->clean))
die("%s: clean filter '%s' failed", path, ca.drv->name);
- crlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
+ crlf_to_git(path, sha1, dst->buf, dst->len, dst, ca.crlf_action,
checksafe);
ident_to_git(path, dst->buf, dst->len, dst, ca.ident);
}
diff --git a/convert.h b/convert.h
index ccf436b..1634d37 100644
--- a/convert.h
+++
[BUG] A part of an edge from an octopus merge gets colored, even with --color=never
With a certain topology involving an octopus merge, git log --graph --oneline --all --color=never produces output which includes some ANSI escape code coloring. Attached is a script to reproduce the problem (creates a git repository in subdir log-format-test), along with sample graph and valgrind output (indicates some unitialialized memory access). I've observed the problem with Windows git versions 2.7.0, 2.5.3. I've NOT observed it with 1.9.5, On GNU/Linux the symptom only appears when running with valgrind, I tried versions 2.8.0, and 2.8.2.402.gedec370 (the last is where the valgrind output comes from) * e98205a c | *[31m-[m[31m.[m 808603b merge a b | |\ \ |/ / / | | * 8886a4e b | * | 2d8743f a | |/ * | e09af19 1 |/ * 773004e 0 test-multiway-merge.sh Description: Bourne shell script ==11287== Memcheck, a memory error detector ==11287== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al. ==11287== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info ==11287== Command: /home/npostavs/src/git/git log --oneline --graph --color=never --all ==11287== ==11287== Conditional jump or move depends on uninitialised value(s) ==11287==at 0x4B0A48: strbuf_write_column (graph.c:79) ==11287==by 0x4B0B3F: graph_draw_octopus_merge (graph.c:799) ==11287==by 0x4B1440: graph_output_commit_line (graph.c:837) ==11287==by 0x4B1726: graph_next_line (graph.c:1126) ==11287==by 0x4B1A5F: graph_show_commit (graph.c:1197) ==11287==by 0x4BCF37: show_log (log-tree.c:601) ==11287==by 0x4BD723: log_tree_commit (log-tree.c:879) ==11287==by 0x441E8C: cmd_log_walk (log.c:345) ==11287==by 0x443961: cmd_log (log.c:660) ==11287==by 0x4050F9: run_builtin (git.c:350) ==11287==by 0x405218: handle_builtin (git.c:536) ==11287==by 0x4055AA: run_argv (git.c:582) ==11287== ==11287== Use of uninitialised value of size 8 ==11287==at 0x4B05A0: column_get_color_code (graph.c:73) ==11287==by 0x4B0A51: strbuf_write_column (graph.c:80) ==11287==by 0x4B0B3F: graph_draw_octopus_merge (graph.c:799) ==11287==by 0x4B1440: graph_output_commit_line (graph.c:837) ==11287==by 0x4B1726: graph_next_line (graph.c:1126) ==11287==by 0x4B1A5F: graph_show_commit (graph.c:1197) ==11287==by 0x4BCF37: show_log (log-tree.c:601) ==11287==by 0x4BD723: log_tree_commit (log-tree.c:879) ==11287==by 0x441E8C: cmd_log_walk (log.c:345) ==11287==by 0x443961: cmd_log (log.c:660) ==11287==by 0x4050F9: run_builtin (git.c:350) ==11287==by 0x405218: handle_builtin (git.c:536) ==11287== ==11287== Conditional jump or move depends on uninitialised value(s) ==11287==at 0x4B0AC5: strbuf_write_column (graph.c:82) ==11287==by 0x4B0B3F: graph_draw_octopus_merge (graph.c:799) ==11287==by 0x4B1440: graph_output_commit_line (graph.c:837) ==11287==by 0x4B1726: graph_next_line (graph.c:1126) ==11287==by 0x4B1A5F: graph_show_commit (graph.c:1197) ==11287==by 0x4BCF37: show_log (log-tree.c:601) ==11287==by 0x4BD723: log_tree_commit (log-tree.c:879) ==11287==by 0x441E8C: cmd_log_walk (log.c:345) ==11287==by 0x443961: cmd_log (log.c:660) ==11287==by 0x4050F9: run_builtin (git.c:350) ==11287==by 0x405218: handle_builtin (git.c:536) ==11287==by 0x4055AA: run_argv (git.c:582) ==11287== ==11287== Conditional jump or move depends on uninitialised value(s) ==11287==at 0x4B0A48: strbuf_write_column (graph.c:79) ==11287==by 0x4B0B6F: graph_draw_octopus_merge (graph.c:802) ==11287==by 0x4B1440: graph_output_commit_line (graph.c:837) ==11287==by 0x4B1726: graph_next_line (graph.c:1126) ==11287==by 0x4B1A5F: graph_show_commit (graph.c:1197) ==11287==by 0x4BCF37: show_log (log-tree.c:601) ==11287==by 0x4BD723: log_tree_commit (log-tree.c:879) ==11287==by 0x441E8C: cmd_log_walk (log.c:345) ==11287==by 0x443961: cmd_log (log.c:660) ==11287==by 0x4050F9: run_builtin (git.c:350) ==11287==by 0x405218: handle_builtin (git.c:536) ==11287==by 0x4055AA: run_argv (git.c:582) ==11287== ==11287== Use of uninitialised value of size 8 ==11287==at 0x4B05A0: column_get_color_code (graph.c:73) ==11287==by 0x4B0A51: strbuf_write_column (graph.c:80) ==11287==by 0x4B0B6F: graph_draw_octopus_merge (graph.c:802) ==11287==by 0x4B1440: graph_output_commit_line (graph.c:837) ==11287==by 0x4B1726: graph_next_line (graph.c:1126) ==11287==by 0x4B1A5F: graph_show_commit (graph.c:1197) ==11287==by 0x4BCF37: show_log (log-tree.c:601) ==11287==by 0x4BD723: log_tree_commit (log-tree.c:879) ==11287==by 0x441E8C: cmd_log_walk (log.c:345) ==11287==by 0x443961: cmd_log (log.c:660) ==11287==by 0x4050F9: run_builtin (git.c:350) ==11287==by 0x405218: handle_builtin (git.c:536) ==11287== ==11287== Conditional jump or move depends on uninitialised value(s) ==11287==at 0x4B0AC5: strbuf_write_column (graph.c:82) ==11287==by 0x4B0B6F: graph_draw_octopus_merge (graph.c:
[GSOC Update] Week 2
= SUMMARY == My micro project on adding config variable to git-commit for verbose options is going to be merged with the master branch soon and will be available for git 2.8.3 . I also rewrote a few shell functions in C. My public git.git is available here[1]. I regularly keep pushing my work so anyone interested can track me there. Feel free to participate in the discussions going on PRs with my mentors. Your comments are valuable. === INTRODUCTION == The purpose of this project is to convert the git-bisect utility which partly exists in the form of shell scripts to C code so as to make it more portable. I plan to do this by converting each function to C and then calling it from git-bisect.sh so as to use the existing test suite to test the function which is converted. Mentors: Christian Couder Lars Schneider == Updates = Things which were done in this week: * I have sent the patches[2] for check_term_format() and write_terms() so as to demonstrate how I am going to use the subcommand approach wherein I will first convert a method and then call it by using a subcommand. Then when another method is converted the previous method will be removed from the subcommand and will instead be called from the new method. Junio has collected this patch and queued it on a branch gitster/pb/bisect and is available for testing on the pu branch. I am encouraging people to test it and provide useful comments. * I have also converted bisect_log() and bisect_voc() whose patches[3] are sent to the list. Junio is yet to pick these up. * I have converted the function bisect_clean_state() but its in a very rudimentary form. Well I generally do like this. I first have a *just* working model of a function and then I polish it by introducing the git's API and write error handling code and resolve the style issues. Though it is available on github[4]. The current version plainly removes the refs using the git's API. I am quite aware that refs shouldn't be handled in this way (this fact is constantly reminded in the docs). I am reading up on the available methods for refs manipulation. * I have also sent an independent patch[5] to explicitly test whether bisection state is properly cleaned up. * Also studied the functioning (upto an extent) of git-for-each-ref and git-udpate ref as it is required during the conversion of bisect_clean_state(). * The main part (I think) was that I read about the method's which handled the refs. It was an interesting read though I did not read upon the actual implementations of those, I mainly covered "What does the method do?" and "How to use the method in my code?". git-grep is my best friend for this. * I am still quite amazed by the amount of attention refs receive. Christian explained a bit though I still don't get the feel why they are *sooo* important. I was recently reading the Git Rev News and also seeing the patches by Michael Haggerty and David Turner and I was quite amazed on the amount of attention refs get. * I also noticed a minor thing with bisect cleanups[6]. After the bisection state is cleaned up the folder "refs/bisect/" is not removed. I pointed this out but Christian thought it is okay the way it is. * When I was converting the function write_terms() Christian advised to use `|| exit` when calling it with a subcommand using `git bisect--helper --write-terms $TERM_BAD bad` but it got the test no. 43 and 44 failing in t6030. On a little bit investigation by Christian, these tests seem to fail when there is a bare repo. He asked me to investigate it further. I have currently not found enough time. Will do it in the coming week. = NEXT STEPS Things which would be done in the coming week: * Finish bisect_clean_state() conversion. I will first put it up on github to receive comments from my mentors and then post it to the mailing list. * Convert the function bisect_head(). I plan to convert this function and add it as a subcommand to test the implementation but I will only send the function without the subcommand to the mailing list because its a too small function. Though the subcommand version will be put up on github for everyone to verify whether it is passing the test suite like I have done it for bisect_voc(). * Convert the function bisect_write(). I plan to convert this function and add it as a subcommand. * Investigate why test no. 43 and 44 are failing in t6030 with `|| exit` in --write-terms. NOTIFICATION == I will be taking a short vacation from 16th May, 2016 to 19th May, 2016 so I wouldn't be available via em
Re: [GSOC Update] Week 2
Pranit Bauva writes: > = SUMMARY == > My micro project on adding config variable to git-commit for verbose options > is going to be merged with the master branch soon and will be available for > git 2.8.3 . Nit: being merged to master doesn't mean you'll be in the next minor release, but in the next _major_ one. 2.8.3 will only contain bugfixes compared to 2.8.2. -- 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 0/4] pathspec labels [WAS: submodule groups]
Duy Nguyen writes: > Instead of putting everything in under the same attribute name > "label", make one attribute per label? Would this work? > > *.[ch] c-group code-group The attribute subsystem expects that there will not be unbounded large number of attributes, so this is not a good direction to go. -- 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 00/94] libify apply and use lib in am
Christian Couder writes: > On Sat, May 14, 2016 at 8:31 PM, Junio C Hamano wrote: >> >> I however do not see a reason why you need to expose that feature to >> the users of "git apply". So I am not sure if any of us care deeply >> the choice among --silent, --quiet and -q -q. > > About that I wrote in initial email above: > ... Because you are not bound to stick to what you thought before the discussion, what you wrote before everybody commented to improve your proposal does not really matter. Otherwise, the time they spent would be wasted. What matters more is what conclusion you drew after having input from others. "That --silent is optional and cannot decide if I want to have the option was what I said at the beginning" does not help the discussion very much at this point. People know that you couldn't decide back then, and that is why they hopefully helped you to get closer to deciding by discussing the series. "OK, I now think we don't need 'apply --silent' because ..., so I'll drop it" or "I still think we want 'apply --silent' because..., so I'll keep it", followed by "thanks for discussing and helping" would be a more appropriate answer, whichever conclusion you have drawn. "Hmm, I still cannot decide." is also fine, by the way. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 'git diff-index' doesn't honor the 'diff.algorithm' variable
Dmitry Gutov writes: > OK, that makes sense. You might want to fix the man page, though, it > says, like the 'git diff' one, "For instance, if you configured > diff.algorithm variable to a non-default value and want to use the > default one, then you have to use --diff-algorithm=default option.". Thanks; I think the paragraph is shared among the "diff" family of commands both plumbing and Porcelain, so I'd say "patches welcome" at this point ;-). > Thanks, but we don't distribute any custom Git porcelains with > Emacs. We usually can't rely on bash being available either. The script was an illustration of the logic--I am sure elisp is much core capable scripting environment than POSIX shell. Perhaps (setq vc-diff-git-diff-use-histogram t) in ~/.emacs is not too bad ;-) > I'll have to see why we using 'git diff-index' there directly. Maybe > we could switch to 'git diff'. I do not think it is a good idea. Depending on how much understanding vc-diff wants to have on what the diff output is saying, it may want to be read and parse parts of the output; an end user who has diff.color=always can easily ruin your day. And no, running "git diff --color=never" is not a solution for that. The Porcelain "git diff" command is not bound by any promise of stable output and reserves the right to change the default to better support human users. I think the upcoming version of Git turns the diff.renames setting on by default, for example. We might even add a side-by-side diff and make it the default someday. You do not want to be reading these "fancy" output, and you cannot keep updating the invocation of "git diff" by vc-diff with unbounded number options, e.g. --no-side-by-side, that will be added to defeat configuration variables that will be invented in the future. -- 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] crlf: Add test showing double warning on commit
Torsten Bögershausen writes: > Nja, (Or Nyes in English), the old handling tried to be "nice" to the user: > $ git add text # gave warning > #User forgets, does other things, git reset HEAD > $ git commit # Gave the warning one more time, to remind the user, > # what he did, and what is really commited. > > But it may be, that diff_populate_filespec() is the wrong place for speaches ? It is *not* necessarily a wrong place for speeches, but certainly it is wrong place to talk about whatever convert_to_git() does. Let's step back and think what the warning is about. I personally feel that "CRLF will be turned into LF" is pointless when the user expressed her preference "I want to have LF in the repository and CRLF in the working tree, when it makes sense". It is not even a warning but merely reports "we did what we were told to do." But for those who set safecrlf=warn to get that warning, we should give the warning when we actually convert the the working tree contents and strip CR from CRLF before recording it as an object. An as-is "git commit" (nothing on command line affects the index with working tree contents, e.g. "-a" or pathspec) does not do any such conversion. The "damage" the user requested to be warned has long been done before the command is run. So the warning from "git add" is good, but it shouldn't appear from anything that does "diff". -- 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: [GSOC Update] Week 2
Pranit Bauva writes: >is available for testing on the pu branch. I am encouraging people to >test it and provide useful comments. Do not encourage people to "TEST". In general, do not put too much weight on testing. The result would only measure a small portion of what you wrote in the code, i.e. what you covered with the addition to the test suite, plus whatever tests we already had. Instead, ask people to review. A new code passing the testsuite is a minimum requirement, and that is far from sufficient. > * I have also converted bisect_log() and bisect_voc() whose patches[3] are >sent to the list. Junio is yet to pick these up. Again, my picking them up is not a success criteria (and certainly being on 'pu' does not count for anything--it is nothing more than "Junio saw them on the list and bookmarked the messages". You should worry more about people not commenting nor reviewing them than me picking them up (which would typically come later). > * The main part (I think) was that I read about the method's which handled > the >refs. It was an interesting read though I did not read upon the actual >implementations of those, I mainly covered "What does the method do?" and >"How to use the method in my code?". git-grep is my best friend for this. Yup. You would not be calling for-each-ref from a C rewrite of bisect-clean-state. Instead you would likely be calling for_each_ref_in() to iterate over the existing refs/bisect/* refs, recording their refname and objectname from the callback to something like string_list, and then after for_each_ref_in() finishes, iterate over the resulting string_list and running delete_ref() on them. And reading the implementation of for-each-ref and update-ref is a good way to find the need to use these API calls and how they are used. API docs are your second step. Overall, good progress for an early week. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSOC Update] Week 2
Matthieu Moy writes: > Pranit Bauva writes: > >> = SUMMARY == >> My micro project on adding config variable to git-commit for verbose options >> is going to be merged with the master branch soon and will be available for >> git 2.8.3 . > > Nit: being merged to master doesn't mean you'll be in the next minor > release, but in the next _major_ one. 2.8.3 will only contain bugfixes > compared to 2.8.2. Correct. Recent "What's cooking" lists the topic as "Will merge to 'master'" but this does not promise in which timeframe it will be merged to 'master', so it may not even be in 2.9.0 at all. Generally, what is in 'next' is marked as "Will merge to 'master'" by default, until a reason not to do so is found, at which time the mark is changed to "Will hold", etc., so saying "master branch soon" upon seeing "Will merge to 'master'" is a bit of overstatement. In fact, "git log maint..pb/commit-verbse-config" shows that this new feature is not ineligible for the maintenance track. It will never be in 2.8.3 or any 2.8.x series. -- 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: [GSOC Update] Week 2
Hey Junio, On Mon, May 16, 2016 at 12:41 AM, Junio C Hamano wrote: > Pranit Bauva writes: > >>is available for testing on the pu branch. I am encouraging people to >>test it and provide useful comments. > > Do not encourage people to "TEST". In general, do not put too much > weight on testing. The result would only measure a small portion of > what you wrote in the code, i.e. what you covered with the addition > to the test suite, plus whatever tests we already had. > > Instead, ask people to review. A new code passing the testsuite is > a minimum requirement, and that is far from sufficient. Okay. Will keep this in mind. >> * I have also converted bisect_log() and bisect_voc() whose patches[3] are >>sent to the list. Junio is yet to pick these up. > > Again, my picking them up is not a success criteria (and certainly > being on 'pu' does not count for anything--it is nothing more than > "Junio saw them on the list and bookmarked the messages". > > You should worry more about people not commenting nor reviewing them > than me picking them up (which would typically come later). Sure. >> * The main part (I think) was that I read about the method's which handled >> the >>refs. It was an interesting read though I did not read upon the actual >>implementations of those, I mainly covered "What does the method do?" and >>"How to use the method in my code?". git-grep is my best friend for this. > > Yup. > > You would not be calling for-each-ref from a C rewrite of > bisect-clean-state. Instead you would likely be calling > for_each_ref_in() to iterate over the existing refs/bisect/* refs, > recording their refname and objectname from the callback to > something like string_list, and then after for_each_ref_in() > finishes, iterate over the resulting string_list and running > delete_ref() on them. Actually I was seeing how for-each-ref called filter_ref() and planning to use that. But for_each_ref_in() seems much better. Thanks. I had planned on using delete_ref(). > And reading the implementation of for-each-ref and update-ref is a > good way to find the need to use these API calls and how they are > used. API docs are your second step. Thanks. I have read the man pages as well as some parts of the implementation (not the core details). API docs contain little information about ref handling though. I can try trying writing some documentation after GSoC project once I am comfortable with ref handling. > Overall, good progress for an early week. Thanks! Regards, Pranit Bauva > Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/4] pathspec labels [WAS: submodule groups]
Junio C Hamano writes: > Duy Nguyen writes: > >> Instead of putting everything in under the same attribute name >> "label", make one attribute per label? Would this work? >> >> *.[ch] c-group code-group > > The attribute subsystem expects that there will not be unbounded > large number of attributes, so this is not a good direction to go. Having said that, I do not mind too much if it turns out that it is necessary to use an unbounded set of random attributes to solve a specific problem, if the use case is good. But even then, in order to avoid confusion and name clashes, I'd prefer to see more like *.[ch] group-c group-code that is matched by git cmd ':(group:c code) i.e. reserve a single prefix that is not and will not be used for other purposes. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git push --quiet option does not seem to work
Try it by making some changes to files and committing them, and then push. It works fine for me when there is nothing to actually push, but not so when there are commits to push. It always outputs the progress to STDERR even when I add --quiet. On Fri, May 13, 2016 at 6:33 PM, Jeff King wrote: > On Fri, May 13, 2016 at 05:21:30PM -0400, Chris B wrote: > >> Hi I am using 2.8.2.windows.1 and writing Powershell scripts doing >> some Git stuff. >> >> I have to use the --quiet option for git because it constantly outputs >> progress to stderr. >> >> However, it seems that --quiet does not actually work in git push. The >> output still goes to stderr. >> When there are changes committed to push it always outputs something >> to stderr. If there is nothing to push, then it actually is silent. > > Can you be more specific? It seems to work for me: > > $ rm -rf dst.git && > git init --bare -q dst.git && > git push dst.git > Counting objects: 6, done. > Delta compression using up to 8 threads. > Compressing objects: 100% (3/3), done. > Writing objects: 100% (6/6), 441 bytes | 0 bytes/s, done. > Total 6 (delta 0), reused 0 (delta 0) > To dst.git >* [new branch] master -> master > > $ rm -rf dst.git && > git init --bare -q dst.git && > git push -q dst.git > [no output] > > Are you seeing progress reporting, the status table, or something else? > Are you using a particular protocol that might invoke a git-remote-* > helper that doesn't respect the quiet flag? > > -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git push --quiet option does not seem to work
By the way, I also notice by your prompt you seem to be testing this in Linux. I did indicate I'm using the Windows version. That might make a difference. On Sun, May 15, 2016 at 4:29 PM, Chris B wrote: > Try it by making some changes to files and committing them, and then push. > It works fine for me when there is nothing to actually push, but not > so when there are commits to push. > > It always outputs the progress to STDERR even when I add --quiet. > > > > On Fri, May 13, 2016 at 6:33 PM, Jeff King wrote: >> On Fri, May 13, 2016 at 05:21:30PM -0400, Chris B wrote: >> >>> Hi I am using 2.8.2.windows.1 and writing Powershell scripts doing >>> some Git stuff. >>> >>> I have to use the --quiet option for git because it constantly outputs >>> progress to stderr. >>> >>> However, it seems that --quiet does not actually work in git push. The >>> output still goes to stderr. >>> When there are changes committed to push it always outputs something >>> to stderr. If there is nothing to push, then it actually is silent. >> >> Can you be more specific? It seems to work for me: >> >> $ rm -rf dst.git && >> git init --bare -q dst.git && >> git push dst.git >> Counting objects: 6, done. >> Delta compression using up to 8 threads. >> Compressing objects: 100% (3/3), done. >> Writing objects: 100% (6/6), 441 bytes | 0 bytes/s, done. >> Total 6 (delta 0), reused 0 (delta 0) >> To dst.git >>* [new branch] master -> master >> >> $ rm -rf dst.git && >> git init --bare -q dst.git && >> git push -q dst.git >> [no output] >> >> Are you seeing progress reporting, the status table, or something else? >> Are you using a particular protocol that might invoke a git-remote-* >> helper that doesn't respect the quiet flag? >> >> -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 'git diff-index' doesn't honor the 'diff.algorithm' variable
On 05/15/2016 09:43 PM, Junio C Hamano wrote: I think the paragraph is shared among the "diff" family of commands both plumbing and Porcelain, so I'd say "patches welcome" at this point ;-). I think I've done my part here. It's not like this is a feature request. The script was an illustration of the logic--I am sure elisp is much core capable scripting environment than POSIX shell. Perhaps (setq vc-diff-git-diff-use-histogram t) in ~/.emacs is not too bad ;-) Yes, doing it via a user option is already possible in Emacs. I was concerned that the user has to configure it twice (once for console, two for Emacs), but if you think it's fine, let's leave it at that. The Porcelain "git diff" command is not bound by any promise of stable output and reserves the right to change the default to better support human users. I think the upcoming version of Git turns the diff.renames setting on by default, for example. We might even add a side-by-side diff and make it the default someday. You do not want to be reading these "fancy" output, and you cannot keep updating the invocation of "git diff" by vc-diff with unbounded number options, e.g. --no-side-by-side, that will be added to defeat configuration variables that will be invented in the future. Fair enough. Thanks, Dmitry. -- 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
Students contributing to Git
Hello Git community, We're a group of french students from Grenoble INP - Ensimag. As part of an academic project, mentored by Matthieu Moy, we chose to spend the four next weeks contributing to Git. We're now picking ideas in order to help the community the most effective way and we're eager to start this adventure ! Kind regards, Jordan De Gea, Samuel Groot, Tom Russello and Erwan Mathonière Grenoble INP - Ensimag students -- 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
Students contributing to Git
Hello Git community, We're a group of french students from Grenoble INP - Ensimag. As part of an academic project, mentored by Matthieu Moy, we chose to spend the four next weeks contributing to Git. We're now picking ideas in order to help the community the most effective way and we're eager to start this adventure ! Kind regards, Jordan De Gea, Samuel Groot, Tom Russello and Erwan Mathonière Grenoble INP - Ensimag students -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git push --quiet option does not seem to work
On Sun, May 15, 2016 at 04:29:36PM -0400, Chris B wrote: > Try it by making some changes to files and committing them, and then push. > It works fine for me when there is nothing to actually push, but not > so when there are commits to push. In my example there were commits to push. As you noted, my test was on Linux, so it's certainly possible that it's Windows-specific. What protocol are you pushing over (e.g., http follows a very different code path for progress)? -Peff -- 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 v1 3/3] convert: ce_compare_data() checks for a sha1 of a path
tbo...@web.de writes:
> -static int has_cr_in_index(const char *path)
> +static int has_cr_in_index(const char *path, const unsigned char *sha1)
> {
> unsigned long sz;
> void *data;
> int has_cr;
> -
> - data = read_blob_data_from_cache(path, &sz);
> - if (!data)
> + enum object_type type;
> + if (!sha1)
> + sha1 = get_sha1_from_cache(path);
> + if (!sha1)
> + return 0;
> + data = read_sha1_file(sha1, &type, &sz);
> + if (!data || type != OBJ_BLOB) {
> + free(data);
> return 0;
> + }
> +
> has_cr = memchr(data, '\r', sz) != NULL;
> free(data);
> return has_cr;
> }
Does this really need 2/3? Wouldn't this be equivalent to
if (!sha1) {
data = read_blob_data_from_cache(path, &sz);
} else {
data = read_sha1_file(sha1, &type, &sz);
}
if (!data || type != OBJ_BLOB) {
free(data);
return 0;
}
has_cr = ...
--
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] commit.c: use strchrnul() to scan for one line
Signed-off-by: Junio C Hamano
---
commit.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/commit.c b/commit.c
index 3f4f371..1f9ee8a 100644
--- a/commit.c
+++ b/commit.c
@@ -415,8 +415,7 @@ int find_commit_subject(const char *commit_buffer, const
char **subject)
p++;
if (*p) {
p += 2;
- for (eol = p; *eol && *eol != '\n'; eol++)
- ; /* do nothing */
+ eol = strchrnul(p, '\n');
} else
eol = p;
--
2.8.2-748-gfb85f76
--
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] attr.c: use strchrnul() to scan for one line
Signed-off-by: Junio C Hamano
---
attr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/attr.c b/attr.c
index a19946a..a7f2c3f 100644
--- a/attr.c
+++ b/attr.c
@@ -407,8 +407,8 @@ static struct attr_stack *read_attr_from_index(const char
*path, int macro_ok)
for (sp = buf; *sp; ) {
char *ep;
int more;
- for (ep = sp; *ep && *ep != '\n'; ep++)
- ;
+
+ ep = strchrnul(sp, '\n');
more = (*ep == '\n');
*ep = '\0';
handle_attr_line(res, sp, path, ++lineno, macro_ok);
--
2.8.2-748-gfb85f76
--
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] attr.c: mark where #if DEBUG ends more clearly
Signed-off-by: Junio C Hamano
---
attr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/attr.c b/attr.c
index a7f2c3f..95416d3 100644
--- a/attr.c
+++ b/attr.c
@@ -469,7 +469,7 @@ static void debug_set(const char *what, const char *match,
struct git_attr *attr
#define debug_push(a) do { ; } while (0)
#define debug_pop(a) do { ; } while (0)
#define debug_set(a,b,c,d) do { ; } while (0)
-#endif
+#endif /* DEBUG_ATTR */
static void drop_attr_stack(void)
{
--
2.8.2-748-gfb85f76
--
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] attr.c: simplify macroexpand_one()
The double-loop wants to do an early return immediately when one
matching macro is found. Eliminate the extra variable 'a' used for
that purpose and rewrite the "assign found itme to 'a' to make it
non-NULL and force the loop(s) to terminate" with a direct return
from there.
Signed-off-by: Junio C Hamano
---
attr.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/attr.c b/attr.c
index 95416d3..7bfeef3 100644
--- a/attr.c
+++ b/attr.c
@@ -701,24 +701,21 @@ static int fill(const char *path, int pathlen, int
basename_offset,
static int macroexpand_one(int nr, int rem)
{
struct attr_stack *stk;
- struct match_attr *a = NULL;
int i;
if (check_all_attr[nr].value != ATTR__TRUE ||
!check_all_attr[nr].attr->maybe_macro)
return rem;
- for (stk = attr_stack; !a && stk; stk = stk->prev)
- for (i = stk->num_matches - 1; !a && 0 <= i; i--) {
+ for (stk = attr_stack; stk; stk = stk->prev) {
+ for (i = stk->num_matches - 1; 0 <= i; i--) {
struct match_attr *ma = stk->attrs[i];
if (!ma->is_macro)
continue;
if (ma->u.attr->attr_nr == nr)
- a = ma;
+ return fill_one("expand", ma, rem);
}
-
- if (a)
- rem = fill_one("expand", a, rem);
+ }
return rem;
}
--
2.8.2-748-gfb85f76
--
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] attr: complete a sentence in a comment
Signed-off-by: Junio C Hamano
---
attr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/attr.c b/attr.c
index aac5c8f..a19946a 100644
--- a/attr.c
+++ b/attr.c
@@ -300,7 +300,7 @@ static struct match_attr *parse_attr_line(const char *line,
const char *src,
* directory (again, reading the file from top to bottom) down to the
* current directory, and then scan the list backwards to find the first match.
* This is exactly the same as what is_excluded() does in dir.c to deal with
- * .gitignore
+ * .gitignore file and info/excludes file as a fallback.
*/
static struct attr_stack {
--
2.8.2-748-gfb85f76
--
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] attr: explain the lack of attr-name syntax check in parse_attr()
Signed-off-by: Junio C Hamano
---
attr.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/attr.c b/attr.c
index 94b27f4..aac5c8f 100644
--- a/attr.c
+++ b/attr.c
@@ -183,6 +183,12 @@ static const char *parse_attr(const char *src, int lineno,
const char *cp,
return NULL;
}
} else {
+ /*
+* As this function is always called twice, once with
+* e == NULL in the first pass and then e != NULL in
+* the second pass, no need for invalid_attr_name()
+* check here.
+*/
if (*cp == '-' || *cp == '!') {
e->setto = (*cp == '-') ? ATTR__FALSE : ATTR__UNSET;
cp++;
--
2.8.2-748-gfb85f76
--
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] attr: update a stale comment on "struct match_attr"
When 82dce998 (attr: more matching optimizations from .gitignore,
2012-10-15) changed a pointer to a string "*pattern" into an
embedded "struct pattern" in struct match_attr, it forgot to update
the comment that describes the structure.
Signed-off-by: Junio C Hamano
---
attr.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/attr.c b/attr.c
index eec5d7d..94b27f4 100644
--- a/attr.c
+++ b/attr.c
@@ -131,9 +131,8 @@ struct pattern {
* If is_macro is true, then u.attr is a pointer to the git_attr being
* defined.
*
- * If is_macro is false, then u.pattern points at the filename pattern
- * to which the rule applies. (The memory pointed to is part of the
- * memory block allocated for the match_attr instance.)
+ * If is_macro is false, then u.pat is the filename pattern to which the
+ * rule applies.
*
* In either case, num_attr is the number of attributes affected by
* this rule, and state is an array listing them. The attributes are
--
2.8.2-748-gfb85f76
--
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 0/4] pathspec labels [WAS: submodule groups]
On Mon, May 16, 2016 at 2:33 AM, Junio C Hamano wrote: > Junio C Hamano writes: > >> Duy Nguyen writes: >> >>> Instead of putting everything in under the same attribute name >>> "label", make one attribute per label? Would this work? >>> >>> *.[ch] c-group code-group >> >> The attribute subsystem expects that there will not be unbounded >> large number of attributes, so this is not a good direction to go. > > Having said that, I do not mind too much if it turns out that it is > necessary to use an unbounded set of random attributes to solve a > specific problem, if the use case is good. I only have a vague idea so far, it seems to me a good idea to be able to specify "binary-marked paths" or "filter attached paths" from pathspec. We can already do this with scripts, but we need to be very careful about quoting. And if it's pathspec it can be combined with other magic (most likely negation) > But even then, in order to avoid confusion and name clashes, I'd > prefer to see more like > > *.[ch] group-c group-code > > that is matched by > > git cmd ':(group:c code) > > i.e. reserve a single prefix that is not and will not be used for > other purposes. For my above use case, i can still define macro group-binary that is an alias of binary to get around this. So it's ok. -- 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
[PATCH v5 7/9] connect: change the --diag-url output to separate user and host
Signed-off-by: Mike Hommey
---
connect.c | 6 ++
t/t5500-fetch-pack.sh | 14 --
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/connect.c b/connect.c
index c40ff35..df15ff3 100644
--- a/connect.c
+++ b/connect.c
@@ -703,10 +703,8 @@ struct child_process *git_connect(int fd[2], const char
*url,
if (flags & CONNECT_DIAG_URL) {
printf("Diag: url=%s\n", url ? url : "NULL");
printf("Diag: protocol=%s\n", prot_name(protocol));
- if (user)
- printf("Diag: userandhost=%s@%s\n", user, host);
- else
- printf("Diag: userandhost=%s\n", host ? host : "NULL");
+ printf("Diag: user=%s\n", user ? user : "NULL");
+ printf("Diag: host=%s\n", host ? host : "NULL");
printf("Diag: port=%s\n", port ? port : "NONE");
printf("Diag: path=%s\n", path ? path : "NULL");
conn = NULL;
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 9acba2b..2d9c4be 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -553,7 +553,7 @@ check_prot_path () {
Diag: protocol=$2
Diag: path=$3
EOF
- git fetch-pack --diag-url "$1" | grep -v host= | grep -v port= >actual
&&
+ git fetch-pack --diag-url "$1" | egrep -v "(user|host|port)=" >actual &&
test_cmp expected actual
}
@@ -568,10 +568,20 @@ check_prot_host_port_path () {
;;
esac
ehost=$(echo $3 | tr -d "[]")
+ case "$ehost" in
+ *@*)
+ user=${ehost%@*}
+ ehost=${ehost#$user@}
+ ;;
+ *)
+ user=NULL
+ ;;
+ esac
cat >exp <<-EOF &&
Diag: url=$1
Diag: protocol=$pp
- Diag: userandhost=$ehost
+ Diag: user=$user
+ Diag: host=$ehost
Diag: port=$4
Diag: path=$5
EOF
--
2.8.2.411.ga331486
--
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 v5 2/9] connect: only match the host with core.gitProxy
Currently, core.gitProxy doesn't actually match purely on domain names as documented: it also matches ports. So a core.gitProxy value like "script for kernel.org" doesn't make the script called for an url like git://kernel.org:port/path, while it is called for git://kernel.org/path. This per-port behavior seems like an oversight rather than a deliberate choice, so, make git://kernel.org:port/path call the gitProxy script in the case described above. Signed-off-by: Mike Hommey --- connect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/connect.c b/connect.c index d3448c2..2a08318 100644 --- a/connect.c +++ b/connect.c @@ -706,7 +706,7 @@ struct child_process *git_connect(int fd[2], const char *url, /* These underlying connection commands die() if they * cannot connect. */ - if (git_use_proxy(hostandport)) + if (git_use_proxy(host)) conn = git_proxy_connect(fd, host, port); else git_tcp_connect(fd, host, port, flags); -- 2.8.2.411.ga331486 -- 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 v5 8/9] connect: actively reject git:// urls with a user part
Currently, urls of the for git://user@host don't work because user@host
is not resolving at the DNS level, but we shouldn't be relying on it
being an invalid host name, and actively reject it for containing a
username in the first place.
Signed-off-by: Mike Hommey
---
connect.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/connect.c b/connect.c
index df15ff3..fdd40b0 100644
--- a/connect.c
+++ b/connect.c
@@ -716,6 +716,9 @@ struct child_process *git_connect(int fd[2], const char
*url,
*/
struct strbuf target_host = STRBUF_INIT;
char *override_vhost = getenv("GIT_OVERRIDE_VIRTUAL_HOST");
+ if (user)
+ die("user@host is not allowed in git:// urls");
+
if (override_vhost)
strbuf_addstr(&target_host, override_vhost);
else {
--
2.8.2.411.ga331486
--
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 v5 0/9] connect: various cleanups
I removed the two controvertial patches, and applied the various suggestions from the last cycle. Mike Hommey (9): connect: call get_host_and_port() earlier connect: only match the host with core.gitProxy connect: fill the host header in the git protocol with the host and port variables connect: make parse_connect_url() return separated host and port connect: group CONNECT_DIAG_URL handling code connect: make parse_connect_url() return the user part of the url as a separate value connect: change the --diag-url output to separate user and host connect: actively reject git:// urls with a user part connect: move ssh command line preparation to a separate function connect.c | 222 -- t/t5500-fetch-pack.sh | 42 +++--- 2 files changed, 157 insertions(+), 107 deletions(-) -- 2.8.2.411.ga331486 -- 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 v5 3/9] connect: fill the host header in the git protocol with the host and port variables
The last use of the hostandport variable, besides being strdup'ed before
being split into host and port, is to fill the host header in the git
protocol.
Instead of relying on parse_connect_url() to return a host:port string
that makes sense there, construct one from the host and port variables.
Signed-off-by: Mike Hommey
---
connect.c | 27 ---
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/connect.c b/connect.c
index 2a08318..ed1a00d 100644
--- a/connect.c
+++ b/connect.c
@@ -695,11 +695,24 @@ struct child_process *git_connect(int fd[2], const char
*url,
* connect, unless the user has overridden us in
* the environment.
*/
- char *target_host = getenv("GIT_OVERRIDE_VIRTUAL_HOST");
- if (target_host)
- target_host = xstrdup(target_host);
- else
- target_host = xstrdup(hostandport);
+ struct strbuf target_host = STRBUF_INIT;
+ char *override_vhost = getenv("GIT_OVERRIDE_VIRTUAL_HOST");
+ if (override_vhost)
+ strbuf_addstr(&target_host, override_vhost);
+ else {
+ /* If the host contains a colon (ipv6 address), it
+* needs to be enclosed with square brackets. */
+ const char *colon = strchr(host, ':');
+ if (colon)
+ strbuf_addch(&target_host, '[');
+ strbuf_addstr(&target_host, host);
+ if (colon)
+ strbuf_addch(&target_host, ']');
+ if (port) {
+ strbuf_addch(&target_host, ':');
+ strbuf_addstr(&target_host, port);
+ }
+ }
transport_check_allowed("git");
@@ -720,8 +733,8 @@ struct child_process *git_connect(int fd[2], const char
*url,
packet_write(fd[1],
"%s %s%chost=%s%c",
prog, path, 0,
-target_host, 0);
- free(target_host);
+target_host.buf, 0);
+ strbuf_release(&target_host);
} else {
conn = xmalloc(sizeof(*conn));
child_process_init(conn);
--
2.8.2.411.ga331486
--
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 v5 4/9] connect: make parse_connect_url() return separated host and port
Now that nothing besides CONNECT_DIAG_URL is using hostandport, we can
have parse_connect_url() itself do the host and port splitting.
This still leaves "user@" part of the host, if there is one, which will
be addressed in a subsequent change. This however does add /some/
handling of the "user@" part of the host, in order to pick the port
properly.
Signed-off-by: Mike Hommey
---
connect.c | 30 ++
t/t5500-fetch-pack.sh | 32 +---
2 files changed, 39 insertions(+), 23 deletions(-)
diff --git a/connect.c b/connect.c
index ed1a00d..3428149 100644
--- a/connect.c
+++ b/connect.c
@@ -589,10 +589,11 @@ static char *get_port(char *host)
* The caller must free() the returned strings.
*/
static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
- char **ret_path)
+ char **ret_port, char **ret_path)
{
char *url;
char *host, *path;
+ const char *port = NULL;
char *end;
int separator = '/';
enum protocol protocol = PROTO_LOCAL;
@@ -647,7 +648,17 @@ static enum protocol parse_connect_url(const char
*url_orig, char **ret_host,
path = xstrdup(path);
*end = '\0';
+ get_host_and_port(&host, &port);
+
+ if (*host && !port) {
+ /* The host might contain a user:password string, ignore it
+* when searching for the port again */
+ char *end_user = strrchr(host, '@');
+ port = get_port(end_user ? end_user : host);
+ }
+
*ret_host = xstrdup(host);
+ *ret_port = port ? xstrdup(port) : NULL;
*ret_path = path;
free(url);
return protocol;
@@ -669,8 +680,7 @@ static struct child_process no_fork = CHILD_PROCESS_INIT;
struct child_process *git_connect(int fd[2], const char *url,
const char *prog, int flags)
{
- char *hostandport, *path, *host;
- const char *port = NULL;
+ char *host, *port, *path;
struct child_process *conn = &no_fork;
enum protocol protocol;
struct strbuf cmd = STRBUF_INIT;
@@ -680,13 +690,12 @@ struct child_process *git_connect(int fd[2], const char
*url,
*/
signal(SIGCHLD, SIG_DFL);
- protocol = parse_connect_url(url, &hostandport, &path);
- host = xstrdup(hostandport);
- get_host_and_port(&host, &port);
+ protocol = parse_connect_url(url, &host, &port, &path);
if ((flags & CONNECT_DIAG_URL) && (protocol != PROTO_SSH)) {
printf("Diag: url=%s\n", url ? url : "NULL");
printf("Diag: protocol=%s\n", prot_name(protocol));
- printf("Diag: hostandport=%s\n", hostandport ? hostandport :
"NULL");
+ printf("Diag: userandhost=%s\n", host ? host : "NULL");
+ printf("Diag: port=%s\n", port ? port : "NONE");
printf("Diag: path=%s\n", path ? path : "NULL");
conn = NULL;
} else if (protocol == PROTO_GIT) {
@@ -752,9 +761,6 @@ struct child_process *git_connect(int fd[2], const char
*url,
int putty = 0, tortoiseplink = 0;
transport_check_allowed("ssh");
- if (!port)
- port = get_port(host);
-
if (flags & CONNECT_DIAG_URL) {
printf("Diag: url=%s\n", url ? url : "NULL");
printf("Diag: protocol=%s\n",
prot_name(protocol));
@@ -762,8 +768,8 @@ struct child_process *git_connect(int fd[2], const char
*url,
printf("Diag: port=%s\n", port ? port : "NONE");
printf("Diag: path=%s\n", path ? path : "NULL");
- free(hostandport);
free(host);
+ free(port);
free(path);
free(conn);
return NULL;
@@ -822,8 +828,8 @@ struct child_process *git_connect(int fd[2], const char
*url,
fd[1] = conn->in; /* write to child's stdin */
strbuf_release(&cmd);
}
- free(hostandport);
free(host);
+ free(port);
free(path);
return conn;
}
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 91a69fc..9acba2b 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -553,7 +553,7 @@ check_prot_path () {
Diag: protocol=$2
Diag: path=$3
EOF
- git fetch-pack --diag-url "$1" | grep -v hostandport= >actual &&
+ git fetch-pack --diag-url "$1" | grep -v host= | grep -v port= >actual
&&
test_cmp expected actual
}
@@ -562,22 +562,17 @@ check_prot_host_port_path () {
case "$2" in
*ssh*)
[PATCH v5 6/9] connect: make parse_connect_url() return the user part of the url as a separate value
Signed-off-by: Mike Hommey
---
connect.c | 33 +
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/connect.c b/connect.c
index 8813f90..c40ff35 100644
--- a/connect.c
+++ b/connect.c
@@ -588,11 +588,13 @@ static char *get_port(char *host)
* 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,
- char **ret_port, char **ret_path)
+static enum protocol parse_connect_url(const char *url_orig, char **ret_user,
+ char **ret_host, char **ret_port,
+ char **ret_path)
{
char *url;
char *host, *path;
+ const char *user = NULL;
const char *port = NULL;
char *end;
int separator = '/';
@@ -650,13 +652,20 @@ static enum protocol parse_connect_url(const char
*url_orig, char **ret_host,
get_host_and_port(&host, &port);
- if (*host && !port) {
+ if (*host) {
/* The host might contain a user:password string, ignore it
* when searching for the port again */
char *end_user = strrchr(host, '@');
- port = get_port(end_user ? end_user : host);
+ if (end_user) {
+ *end_user = '\0';
+ user = host;
+ host = end_user + 1;
+ }
}
+ if (!port)
+ port = get_port(host);
+ *ret_user = user ? xstrdup(user) : NULL;
*ret_host = xstrdup(host);
*ret_port = port ? xstrdup(port) : NULL;
*ret_path = path;
@@ -680,7 +689,7 @@ static struct child_process no_fork = CHILD_PROCESS_INIT;
struct child_process *git_connect(int fd[2], const char *url,
const char *prog, int flags)
{
- char *host, *port, *path;
+ char *user, *host, *port, *path;
struct child_process *conn = &no_fork;
enum protocol protocol;
struct strbuf cmd = STRBUF_INIT;
@@ -690,11 +699,14 @@ struct child_process *git_connect(int fd[2], const char
*url,
*/
signal(SIGCHLD, SIG_DFL);
- protocol = parse_connect_url(url, &host, &port, &path);
+ protocol = parse_connect_url(url, &user, &host, &port, &path);
if (flags & CONNECT_DIAG_URL) {
printf("Diag: url=%s\n", url ? url : "NULL");
printf("Diag: protocol=%s\n", prot_name(protocol));
- printf("Diag: userandhost=%s\n", host ? host : "NULL");
+ if (user)
+ printf("Diag: userandhost=%s@%s\n", user, host);
+ else
+ printf("Diag: userandhost=%s\n", host ? host : "NULL");
printf("Diag: port=%s\n", port ? port : "NONE");
printf("Diag: path=%s\n", path ? path : "NULL");
conn = NULL;
@@ -801,7 +813,11 @@ struct child_process *git_connect(int fd[2], const char
*url,
argv_array_push(&conn->args, putty ? "-P" :
"-p");
argv_array_push(&conn->args, port);
}
- argv_array_push(&conn->args, host);
+ if (user)
+ argv_array_pushf(&conn->args, "%s@%s",
+user, host);
+ else
+ argv_array_push(&conn->args, host);
} else {
transport_check_allowed("file");
}
@@ -814,6 +830,7 @@ struct child_process *git_connect(int fd[2], const char
*url,
fd[1] = conn->in; /* write to child's stdin */
strbuf_release(&cmd);
}
+ free(user);
free(host);
free(port);
free(path);
--
2.8.2.411.ga331486
--
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 v5 5/9] connect: group CONNECT_DIAG_URL handling code
Previous changes made both branches handling CONNECT_DIAG_URL identical.
We can now remove one of those branches and have CONNECT_DIAG_URL be
handled in one place.
Signed-off-by: Mike Hommey
---
connect.c | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/connect.c b/connect.c
index 3428149..8813f90 100644
--- a/connect.c
+++ b/connect.c
@@ -691,7 +691,7 @@ struct child_process *git_connect(int fd[2], const char
*url,
signal(SIGCHLD, SIG_DFL);
protocol = parse_connect_url(url, &host, &port, &path);
- if ((flags & CONNECT_DIAG_URL) && (protocol != PROTO_SSH)) {
+ if (flags & CONNECT_DIAG_URL) {
printf("Diag: url=%s\n", url ? url : "NULL");
printf("Diag: protocol=%s\n", prot_name(protocol));
printf("Diag: userandhost=%s\n", host ? host : "NULL");
@@ -761,20 +761,6 @@ struct child_process *git_connect(int fd[2], const char
*url,
int putty = 0, tortoiseplink = 0;
transport_check_allowed("ssh");
- if (flags & CONNECT_DIAG_URL) {
- printf("Diag: url=%s\n", url ? url : "NULL");
- printf("Diag: protocol=%s\n",
prot_name(protocol));
- printf("Diag: userandhost=%s\n", host ? host :
"NULL");
- printf("Diag: port=%s\n", port ? port : "NONE");
- printf("Diag: path=%s\n", path ? path : "NULL");
-
- free(host);
- free(port);
- free(path);
- free(conn);
- return NULL;
- }
-
ssh = getenv("GIT_SSH_COMMAND");
if (!ssh) {
const char *base;
--
2.8.2.411.ga331486
--
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 v5 1/9] connect: call get_host_and_port() earlier
Currently, get_host_and_port() is called in git_connect() for the ssh
protocol, and in git_tcp_connect_sock() for the git protocol. Instead
of doing this, just call it from a single place, right after
parse_connect_url(), and pass the host and port separately to
git_*_connect() functions.
We however preserve hostandport, at least for now.
Note that in git_tcp_connect_sock, the port was set to "" in a
case that never can happen, so that code path was removed.
Signed-off-by: Mike Hommey
---
connect.c | 47 +++
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/connect.c b/connect.c
index c53f3f1..d3448c2 100644
--- a/connect.c
+++ b/connect.c
@@ -343,18 +343,16 @@ static const char *ai_name(const struct addrinfo *ai)
/*
* Returns a connected socket() fd, or else die()s.
*/
-static int git_tcp_connect_sock(char *host, int flags)
+static int git_tcp_connect_sock(const char *host, const char *port, int flags)
{
struct strbuf error_message = STRBUF_INIT;
int sockfd = -1;
- const char *port = STR(DEFAULT_GIT_PORT);
struct addrinfo hints, *ai0, *ai;
int gai;
int cnt = 0;
- get_host_and_port(&host, &port);
- if (!*port)
- port = "";
+ if (!port)
+ port = STR(DEFAULT_GIT_PORT);
memset(&hints, 0, sizeof(hints));
if (flags & CONNECT_IPV4)
@@ -411,11 +409,10 @@ static int git_tcp_connect_sock(char *host, int flags)
/*
* Returns a connected socket() fd, or else die()s.
*/
-static int git_tcp_connect_sock(char *host, int flags)
+static int git_tcp_connect_sock(const char *host, const char *port, int flags)
{
struct strbuf error_message = STRBUF_INIT;
int sockfd = -1;
- const char *port = STR(DEFAULT_GIT_PORT);
char *ep;
struct hostent *he;
struct sockaddr_in sa;
@@ -423,7 +420,8 @@ static int git_tcp_connect_sock(char *host, int flags)
unsigned int nport;
int cnt;
- get_host_and_port(&host, &port);
+ if (!port)
+ port = STR(DEFAULT_GIT_PORT);
if (flags & CONNECT_VERBOSE)
fprintf(stderr, "Looking up %s ... ", host);
@@ -482,9 +480,10 @@ static int git_tcp_connect_sock(char *host, int flags)
#endif /* NO_IPV6 */
-static void git_tcp_connect(int fd[2], char *host, int flags)
+static void git_tcp_connect(int fd[2], const char *host, const char *port,
+ int flags)
{
- int sockfd = git_tcp_connect_sock(host, flags);
+ int sockfd = git_tcp_connect_sock(host, port, flags);
fd[0] = sockfd;
fd[1] = dup(sockfd);
@@ -550,18 +549,16 @@ static int git_use_proxy(const char *host)
return (git_proxy_command && *git_proxy_command);
}
-static struct child_process *git_proxy_connect(int fd[2], char *host)
+static struct child_process *git_proxy_connect(int fd[2], const char *host,
+ const char *port)
{
- const char *port = STR(DEFAULT_GIT_PORT);
struct child_process *proxy;
- get_host_and_port(&host, &port);
-
proxy = xmalloc(sizeof(*proxy));
child_process_init(proxy);
argv_array_push(&proxy->args, git_proxy_command);
argv_array_push(&proxy->args, host);
- argv_array_push(&proxy->args, port);
+ argv_array_push(&proxy->args, port ? port : STR(DEFAULT_GIT_PORT));
proxy->in = -1;
proxy->out = -1;
if (start_command(proxy))
@@ -672,7 +669,8 @@ static struct child_process no_fork = CHILD_PROCESS_INIT;
struct child_process *git_connect(int fd[2], const char *url,
const char *prog, int flags)
{
- char *hostandport, *path;
+ char *hostandport, *path, *host;
+ const char *port = NULL;
struct child_process *conn = &no_fork;
enum protocol protocol;
struct strbuf cmd = STRBUF_INIT;
@@ -683,6 +681,8 @@ struct child_process *git_connect(int fd[2], const char
*url,
signal(SIGCHLD, SIG_DFL);
protocol = parse_connect_url(url, &hostandport, &path);
+ host = xstrdup(hostandport);
+ get_host_and_port(&host, &port);
if ((flags & CONNECT_DIAG_URL) && (protocol != PROTO_SSH)) {
printf("Diag: url=%s\n", url ? url : "NULL");
printf("Diag: protocol=%s\n", prot_name(protocol));
@@ -707,9 +707,9 @@ struct child_process *git_connect(int fd[2], const char
*url,
* cannot connect.
*/
if (git_use_proxy(hostandport))
- conn = git_proxy_connect(fd, hostandport);
+ conn = git_proxy_connect(fd, host, port);
else
- git_tcp_connect(fd, hostandport, flags);
+ git_tcp_connect(fd, host, port, flags);
/*
* Separate original protocol components prog and
[PATCH v5 9/9] connect: move ssh command line preparation to a separate function
Signed-off-by: Mike Hommey
---
connect.c | 108 +-
1 file changed, 58 insertions(+), 50 deletions(-)
diff --git a/connect.c b/connect.c
index fdd40b0..ae9ef7b 100644
--- a/connect.c
+++ b/connect.c
@@ -673,6 +673,61 @@ static enum protocol parse_connect_url(const char
*url_orig, char **ret_user,
return protocol;
}
+static int prepare_ssh_command(struct argv_array *cmd, const char *user,
+ const char *host, const char *port, int flags)
+{
+ const char *ssh;
+ int putty = 0, tortoiseplink = 0, use_shell = 1;
+ transport_check_allowed("ssh");
+
+ ssh = getenv("GIT_SSH_COMMAND");
+ if (!ssh) {
+ const char *base;
+ char *ssh_dup;
+
+ /*
+* GIT_SSH is the no-shell version of
+* GIT_SSH_COMMAND (and must remain so for
+* historical compatibility).
+*/
+ use_shell = 0;
+
+ ssh = getenv("GIT_SSH");
+ if (!ssh)
+ ssh = "ssh";
+
+ ssh_dup = xstrdup(ssh);
+ base = basename(ssh_dup);
+
+ tortoiseplink = !strcasecmp(base, "tortoiseplink") ||
+ !strcasecmp(base, "tortoiseplink.exe");
+ putty = tortoiseplink ||
+ !strcasecmp(base, "plink") ||
+ !strcasecmp(base, "plink.exe");
+
+ free(ssh_dup);
+ }
+
+ argv_array_push(cmd, ssh);
+ if (flags & CONNECT_IPV4)
+ argv_array_push(cmd, "-4");
+ else if (flags & CONNECT_IPV6)
+ argv_array_push(cmd, "-6");
+ if (tortoiseplink)
+ argv_array_push(cmd, "-batch");
+ if (port) {
+ /* P is for PuTTY, p is for OpenSSH */
+ argv_array_push(cmd, putty ? "-P" : "-p");
+ argv_array_push(cmd, port);
+ }
+ if (user)
+ argv_array_pushf(cmd, "%s@%s", user, host);
+ else
+ argv_array_push(cmd, host);
+
+ return use_shell;
+}
+
static struct child_process no_fork = CHILD_PROCESS_INIT;
/*
@@ -767,59 +822,12 @@ struct child_process *git_connect(int fd[2], const char
*url,
/* remove repo-local variables from the environment */
conn->env = local_repo_env;
- conn->use_shell = 1;
conn->in = conn->out = -1;
if (protocol == PROTO_SSH) {
- const char *ssh;
- int putty = 0, tortoiseplink = 0;
- transport_check_allowed("ssh");
-
- ssh = getenv("GIT_SSH_COMMAND");
- if (!ssh) {
- const char *base;
- char *ssh_dup;
-
- /*
-* GIT_SSH is the no-shell version of
-* GIT_SSH_COMMAND (and must remain so for
-* historical compatibility).
-*/
- conn->use_shell = 0;
-
- ssh = getenv("GIT_SSH");
- if (!ssh)
- ssh = "ssh";
-
- ssh_dup = xstrdup(ssh);
- base = basename(ssh_dup);
-
- tortoiseplink = !strcasecmp(base,
"tortoiseplink") ||
- !strcasecmp(base, "tortoiseplink.exe");
- putty = tortoiseplink ||
- !strcasecmp(base, "plink") ||
- !strcasecmp(base, "plink.exe");
-
- free(ssh_dup);
- }
-
- argv_array_push(&conn->args, ssh);
- if (flags & CONNECT_IPV4)
- argv_array_push(&conn->args, "-4");
- else if (flags & CONNECT_IPV6)
- argv_array_push(&conn->args, "-6");
- if (tortoiseplink)
- argv_array_push(&conn->args, "-batch");
- if (port) {
- /* P is for PuTTY, p is for OpenSSH */
- argv_array_push(&conn->args, putty ? "-P" :
"-p");
- argv_array_push(&conn->args, port);
- }
- if (user)
- argv_array_pushf(&conn->args, "%s@%s",
-user, host);
- else
- argv_array_push(&conn->args, host);
+ conn->use_shell = prepare_ssh_command(
+
Re: [PATCH 4/5] worktree: add "lock" command
On Tue, May 10, 2016 at 10:17 AM, Nguyễn Thái Ngọc Duy
wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -11,6 +11,7 @@ SYNOPSIS
> [verse]
> 'git worktree add' [-f] [--detach] [--checkout] [-b ]
> []
> 'git worktree list' [--porcelain]
> +'git worktree lock' [--reason ]
> 'git worktree prune' [-n] [-v] [--expire ]
>
> DESCRIPTION
This forgets to update the paragraph in DESCRIPTION which currently
talks about manually creating the 'locked' file. Perhaps it can be
rewritten like this:
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 issuing the `git
worktree lock` command, optionally specifying `--reason` to
explain why the working tree is locked.
The DETAILS section might also need a small update. It currently says:
...add a file named 'locked'...
but perhaps it should instead say:
...use the `git worktree lock` command, which adds a
file named `locked`,...
> @@ -61,6 +62,12 @@ each of the linked worktrees. The output details include
> if the worktree is
> +lock::
> +
> +When a worktree is locked, it cannot be pruned, moved or deleted. For
> +example, if the worktree is on portable device that is not available
> +when "git worktree " is executed.
The really important reason for locking a worktree is to prevent its
administrative files from being pruned *automatically*. This
description doesn't properly emphasize that point.
Also, bc48328 (Documentation/git-worktree: consistently use term
"linked working tree", 2015-07-20) comprehensively replaced the term
"worktree" with "working tree" and, although some new instances
slipped in with bb9c03b (worktree: add 'list' command, 2015-10-08), we
should probably avoid adding more.
Perhaps the description of "lock" can be rewritten like this:
If a working tree is on a portable device or network share which
is not always mounted, lock it to prevent its administrative
files from being pruned automatically. This also prevents it from
being moved or deleted. Optionally, specify a reason for the lock
with `--reason`.
I guess preventing it from being deleted when locked is a safety measure?
> @@ -110,6 +117,9 @@ OPTIONS
> --expire ::
> With `prune`, only expire unused working trees older than .
>
> +--reason :
> + An explanation why the worktree is locked.
For consistency with other option descriptions, perhaps:
With `lock`, an explanation ...
though I don't feel strongly about it.
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -459,6 +460,44 @@ static int list(int ac, const char **av, const char
> *prefix)
> +static int lock_worktree(int ac, const char **av, const char *prefix)
> +{
> + const char *reason = "", *old_reason;
> + struct option options[] = {
> + OPT_STRING(0, "reason", &reason, N_("string"),
> + N_("reason for locking")),
> + OPT_END()
> + };
> + struct worktree **worktrees, *wt;
> + struct strbuf dst = STRBUF_INIT;
> +
> + ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
> + if (ac != 1)
> + usage_with_options(worktree_usage, options);
> +
> + strbuf_addstr(&dst, prefix_filename(prefix,
> + strlen(prefix),
> + av[0]));
> +
> + worktrees = get_worktrees();
> + wt = find_worktree_by_path(worktrees, dst.buf);
> + if (!wt)
> + die(_("'%s' is not a working directory"), av[0]);
> + if (is_main_worktree(wt))
> + die(_("'%s' is a main working directory"), av[0]);
> +
> + old_reason = is_worktree_locked(wt);
> + if (old_reason) {
> + if (*old_reason)
> + die(_("already locked, reason: %s"), old_reason);
> + die(_("already locked, no reason"));
> + }
As a convenience, would it make sense to allow a worktree to be
re-locked with a different reason rather than erroring out?
Also, the "no reason" in the error message might be confusing. Perhaps
it would be better to say merely "already locked" in this case.
> + write_file(git_common_path("worktrees/%s/locked", wt->id),
> + "%s", reason);
> + return 0;
> +}
This is a tangent, but it would be nice for the "git worktree list"
command to show whether a worktree is locked, and the reason (if
available), both in pretty and porcelain formats. (That was another
reason why I suggested to Mike, back when he was adding the "list"
command, that 'struct worktree' should have a 'locked' field and
get_worktrees() should populate it automatically.)
> diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +test_descripti
Re: [PATCH v5 0/2] bisect--helper: rewrite of check-term-format()
On Sun, May 8, 2016 at 9:34 AM, Pranit Bauva wrote: > On Sun, May 8, 2016 at 11:53 AM, Pranit Bauva wrote: >> On Sun, May 8, 2016 at 7:55 AM, Junio C Hamano wrote: >>> Pranit Bauva writes: I completely missed your point and you want me to go the Eric Sunshine's way? >>> >>> I am neutral. >>> >>> When I read your response to Eric's "top down" suggestion, I didn't >>> quite get much more than "I started going this way, and I do not >>> want to change to the other direction.", which was what I had the >>> most trouble with. If your justification for the approach to start >>> from building a tiny bottom layer that will need to be dismantled >>> soon and repeat that (which sounds somewhat wasteful) were more >>> convincing, I may have felt differently. >> >> Sorry if it seemed that "I have done quite some work and I don't want >> to scrape it off and redo everything". This isn't a case for me. I >> think of this as just a small part in the process of learning and my >> efforts would be completely wasted as I can still reuse the methods I > > efforts would **not** be completely wasted > >> wrote. This is still open for a "philosophical" discussion. I am >> assuming 1e1ea69fa4e is how Eric is suggesting. Speaking of 1e1ea69 (pull: implement skeletal builtin pull, 2015-06-14), one of the (numerous) things Paul Tan did which impressed me was to formally measure test suite coverage of the commands he was converting to C, and then improve coverage where it was lacking. That approach increases confidence in the conversion far more than fallible human reviews do. Setting aside the top-down vs. bottom-up discussion, as a reviewer (and user) I'd be far more interested in seeing you spend a good initial chunk of your project emulating Paul's approach to measuring and improving test coverage (though I don't know how your GSoC mentors feel about 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 4/5] worktree: add "lock" command
On Tue, May 10, 2016 at 10:17 AM, Nguyễn Thái Ngọc Duy
wrote:
> +static int lock_worktree(int ac, const char **av, const char *prefix)
> +{
> + const char *reason = "", *old_reason;
> + struct option options[] = {
> + OPT_STRING(0, "reason", &reason, N_("string"),
> + N_("reason for locking")),
> + OPT_END()
> + };
> + struct worktree **worktrees, *wt;
> + struct strbuf dst = STRBUF_INIT;
> +
> + ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
> + if (ac != 1)
> + usage_with_options(worktree_usage, options);
> +
> + strbuf_addstr(&dst, prefix_filename(prefix,
> + strlen(prefix),
> + av[0]));
> +
> + worktrees = get_worktrees();
> + wt = find_worktree_by_path(worktrees, dst.buf);
Oh, I forgot to mention in my previous email that this is leaking
'strbuf dst'. It could be released right here at its point of final
use.
> + if (!wt)
> + die(_("'%s' is not a working directory"), av[0]);
> + if (is_main_worktree(wt))
> + die(_("'%s' is a main working directory"), av[0]);
> +
> + old_reason = is_worktree_locked(wt);
> + if (old_reason) {
> + if (*old_reason)
> + die(_("already locked, reason: %s"), old_reason);
> + die(_("already locked, no reason"));
> + }
> +
> + write_file(git_common_path("worktrees/%s/locked", wt->id),
> + "%s", reason);
> + 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 5/5] worktree: add "unlock" command
On Tue, May 10, 2016 at 10:17 AM, Nguyễn Thái Ngọc Duy
wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -498,6 +499,34 @@ static int lock_worktree(int ac, const char **av, const
> char *prefix)
> +static int unlock_worktree(int ac, const char **av, const char *prefix)
> +{
> + struct option options[] = {
> + OPT_END()
> + };
> + struct worktree **worktrees, *wt;
> + struct strbuf dst = STRBUF_INIT;
> +
> + ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
> + if (ac != 1)
> + usage_with_options(worktree_usage, options);
> +
> + strbuf_addstr(&dst, prefix_filename(prefix,
> + strlen(prefix),
> + av[0]));
> +
> + worktrees = get_worktrees();
> + wt = find_worktree_by_path(worktrees, dst.buf);
strbuf_release(&dst);
> + if (!wt)
> + die(_("'%s' is not a working directory"), av[0]);
> + if (is_main_worktree(wt))
> + die(_("'%s' is a main working directory"), av[0]);
> + if (!is_worktree_locked(wt))
> + die(_("not locked"));
> +
> + return unlink_or_warn(git_common_path("worktrees/%s/locked", wt->id));
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git push --quiet option does not seem to work
I did not see in your example any commit. But if you say so. I saw git init which would be a new repo.. don't know if it makes a difference. It's pushing to HTTPS. I can provide the real example tomorrow. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git push --quiet option does not seem to work
On Sun, May 15, 2016 at 08:51:53PM -0400, Chris B wrote: > I did not see in your example any commit. But if you say so. I didn't show the commit step. But you can see that I made two identical pushes, one quiet and one not, and the non-quiet one actually pushed commits and showed the progress meter. That doesn't help with your problem, though... > I saw git init which would be a new repo.. don't know if it makes a > difference. > > It's pushing to HTTPS. I repeated my test pushing over https, but it still seems to work as expected with "-q". So perhaps it is just Windows-specific, or perhaps there's something else going on. -Peff -- 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 52/94] builtin/apply: read_patch_file() return -1 instead of die()ing
On Wed, May 11, 2016 at 9:17 AM, Christian Couder
wrote:
> To libify `git apply` functionality we have to signal errors to the
> caller instead of die()ing. Let's do that by using error() instead
> of die()ing in read_patch_file().
>
> Signed-off-by: Christian Couder
> ---
> diff --git a/builtin/apply.c b/builtin/apply.c
> @@ -445,10 +445,10 @@ static void say_patch_name(FILE *output, const char
> *fmt, struct patch *patch)
> -static void read_patch_file(struct strbuf *sb, int fd)
> +static int read_patch_file(struct strbuf *sb, int fd)
> {
> if (strbuf_read(sb, fd, 0) < 0)
> - die_errno("git apply: failed to read");
> + return error("git apply: failed to read: %s",
> strerror(errno));
When Duy's nd/error-errno series, which is in 'next', gets promoted to
'master', then this could become:
return error_errno(...);
> /*
> * Make sure that we have some slop in the buffer
> @@ -457,6 +457,7 @@ static void read_patch_file(struct strbuf *sb, int fd)
> */
> strbuf_grow(sb, SLOP);
> memset(sb->buf + sb->len, 0, SLOP);
> + 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
[PATCH] Ignore dirty submodule states during stash
Do not save states of submodules as stash should ignore it.
Signed-off-by: Vasily Titskiy
---
git-stash.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-stash.sh b/git-stash.sh
index c7c65e2..b500c44 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -116,7 +116,7 @@ create_stash () {
git read-tree --index-output="$TMPindex" -m $i_tree &&
GIT_INDEX_FILE="$TMPindex" &&
export GIT_INDEX_FILE &&
- git diff --name-only -z HEAD -- >"$TMP-stagenames" &&
+ git diff --name-only --ignore-submodules -z HEAD --
>"$TMP-stagenames" &&
git update-index -z --add --remove --stdin
<"$TMP-stagenames" &&
git write-tree &&
rm -f "$TMPindex"
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 54/94] builtin/apply: make parse_chunk() return a negative integer on error
On Wed, May 11, 2016 at 9:17 AM, Christian Couder
wrote:
> To libify `git apply` functionality we have to signal errors to the
> caller instead of die()ing or exit()ing.
>
> To do that in a compatible manner with the rest of the error handling
> in builtin/apply.c, find_header() should return -1 instead of calling
> die() or exit().
Why is this talking about making find_header() return -1? Didn't that
happen in the previous patch?
> As parse_chunk() is called only by apply_patch() which already
> returns -1 when an error happened, let's make apply_patch() return -1
> when parse_chunk() returns -1.
>
> If find_header() returns -2 because no patch header has been found, it
> is ok for parse_chunk() to also return -2. If find_header() returns -1
> because an error happened, it is ok for parse_chunk() to do the same.
>
> Helped-by: Eric Sunshine
> Signed-off-by: Christian Couder
> ---
> diff --git a/builtin/apply.c b/builtin/apply.c
> @@ -2176,8 +2176,9 @@ static int parse_chunk(struct apply_state *state, char
> *buffer, unsigned long si
> * empty to us here.
> */
> if ((state->apply || state->check) &&
> - (!patch->is_binary && !metadata_changes(patch)))
> - die(_("patch with only garbage at line %d"),
> state->linenr);
> + (!patch->is_binary && !metadata_changes(patch))) {
> + return error(_("patch with only garbage at line %d"),
> state->linenr);
> + }
Unnecessary braces.
> }
>
> return offset + hdrsize + patchsize;
--
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 56/94] apply: move 'struct apply_state' to apply.h
On Wed, May 11, 2016 at 9:17 AM, Christian Couder wrote: > To libify `git apply` functionality we must make 'struct apply_state' > usable outside "builtin/apply.c". Why is this patch plopped right in the middle of a bunch of other patches which are making functions return -1 rather than die()ing? Seems out of place. > Let's do that by creating a new "apply.h" and moving > 'struct apply_state' there. I could easily see this header introduced at a very early stage when 'struct apply_state' itself is introduced, rather than introducing 'struct apply_state' in builtin/apply.c and later moving it here. > Signed-off-by: Christian Couder -- 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 59/94] builtin/apply: move init_apply_state() to apply.c
On Wed, May 11, 2016 at 9:17 AM, Christian Couder
wrote:
> To libify `git apply` functionality we must make init_apply_state()
> usable outside "builtin/apply.c".
>
> Let's do that by moving it into a new "apply.c".
Similar to my comment about apply.h and 'struct apply_state', I can
easily see apply.c introduced very early in the conversion. In fact,
I'd imagine apply.c and apply.h being introduced by the same patch,
with that patch placing init_apply_state() in apply.c and 'struct
apply_state' in apply.h. However, I see that you're moving other
functions, as well, which already existed in builtin/apply.c, so I
guess that's why introduction of apply.c waited until this step.
> Helped-by: Eric Sunshine
> Signed-off-by: Christian Couder
> ---
> diff --git a/apply.c b/apply.c
> @@ -0,0 +1,83 @@
> +#include "cache.h"
> +#include "lockfile.h"
> +#include "apply.h"
> +
> +static void git_apply_config(void)
> +{
> + git_config_get_string_const("apply.whitespace",
> &apply_default_whitespace);
> + git_config_get_string_const("apply.ignorewhitespace",
> &apply_default_ignorewhitespace);
> + git_config(git_default_config, NULL);
> +}
> +
> +int parse_whitespace_option(struct apply_state *state, const char *option)
> +{
> + if (!option) {
> + state->ws_error_action = warn_on_ws_error;
> + return 0;
> + }
> + if (!strcmp(option, "warn")) {
> + state->ws_error_action = warn_on_ws_error;
> + return 0;
> + }
> + if (!strcmp(option, "nowarn")) {
> + state->ws_error_action = nowarn_ws_error;
> + return 0;
> + }
> + if (!strcmp(option, "error")) {
> + state->ws_error_action = die_on_ws_error;
> + return 0;
> + }
> + if (!strcmp(option, "error-all")) {
> + state->ws_error_action = die_on_ws_error;
> + state->squelch_whitespace_errors = 0;
> + return 0;
> + }
> + if (!strcmp(option, "strip") || !strcmp(option, "fix")) {
> + state->ws_error_action = correct_ws_error;
> + return 0;
> + }
> + return error(_("unrecognized whitespace option '%s'"), option);
> +}
> +
> +int parse_ignorewhitespace_option(struct apply_state *state,
> + const char *option)
> +{
> + if (!option || !strcmp(option, "no") ||
> + !strcmp(option, "false") || !strcmp(option, "never") ||
> + !strcmp(option, "none")) {
> + state->ws_ignore_action = ignore_ws_none;
> + return 0;
> + }
> + if (!strcmp(option, "change")) {
> + state->ws_ignore_action = ignore_ws_change;
> + return 0;
> + }
> + return error(_("unrecognized whitespace ignore option '%s'"), option);
> +}
> +
> +void init_apply_state(struct apply_state *state,
> + const char *prefix,
> + struct lock_file *lock_file)
> +{
> + memset(state, 0, sizeof(*state));
> + state->prefix = prefix;
> + state->prefix_length = state->prefix ? strlen(state->prefix) : 0;
> + state->lock_file = lock_file ? lock_file : xcalloc(1,
> sizeof(*lock_file));
> + state->newfd = -1;
> + state->apply = 1;
> + state->line_termination = '\n';
> + state->p_value = 1;
> + state->p_context = UINT_MAX;
> + state->squelch_whitespace_errors = 5;
> + state->ws_error_action = warn_on_ws_error;
> + state->ws_ignore_action = ignore_ws_none;
> + state->linenr = 1;
> + strbuf_init(&state->root, 0);
> +
> + git_apply_config();
> + if (apply_default_whitespace && parse_whitespace_option(state,
> apply_default_whitespace))
> + exit(1);
> + if (apply_default_ignorewhitespace &&
> parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
> + exit(1);
> +}
> +
> diff --git a/apply.h b/apply.h
> index aa11ea6..0f77f4d 100644
> --- a/apply.h
> +++ b/apply.h
> @@ -112,4 +112,13 @@ struct apply_state {
> enum ws_ignore ws_ignore_action;
> };
>
> +extern int parse_whitespace_option(struct apply_state *state,
> + const char *option);
> +extern int parse_ignorewhitespace_option(struct apply_state *state,
> +const char *option);
> +
> +extern void init_apply_state(struct apply_state *state,
> +const char *prefix,
> +struct lock_file *lock_file);
> +
> #endif
--
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] attr.c: simplify macroexpand_one()
On Sun, May 15, 2016 at 6:57 PM, Junio C Hamano wrote: > The double-loop wants to do an early return immediately when one > matching macro is found. Eliminate the extra variable 'a' used for > that purpose and rewrite the "assign found itme to 'a' to make it What's "itme"? > non-NULL and force the loop(s) to terminate" with a direct return > from there. > > Signed-off-by: Junio C Hamano -- 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 60/94] apply: make init_apply_state() return -1 instead of exit()ing
On Wed, May 11, 2016 at 9:17 AM, Christian Couder
wrote:
> To libify `git apply` functionality we have to signal errors to the
> caller instead of exit()ing.
>
> To do that in a compatible manner with the rest of the error handling
> in "builtin/apply.c", init_apply_state() should return -1 using
> error() instead of calling exit().
This commit message seems to be lying. It says that the -1 comes from
error(), however, init_apply_state() just returns literal -1 in all
cases.
By the way, mentioning "return -1 using error()" in all of these
commit messages (not just this one) may be overkill. The fact that
that side-effect of error() is being used in some of these cases is an
implementation detail which doesn't necessarily merit mention in the
commit message.
> Signed-off-by: Christian Couder
> ---
> diff --git a/apply.c b/apply.c
> index 508ea64..1e2b802 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -55,9 +55,9 @@ int parse_ignorewhitespace_option(struct apply_state *state,
> return error(_("unrecognized whitespace ignore option '%s'"), option);
> }
>
> -void init_apply_state(struct apply_state *state,
> - const char *prefix,
> - struct lock_file *lock_file)
> +int init_apply_state(struct apply_state *state,
> +const char *prefix,
> +struct lock_file *lock_file)
> {
> memset(state, 0, sizeof(*state));
> state->prefix = prefix;
> @@ -76,8 +76,9 @@ void init_apply_state(struct apply_state *state,
>
> git_apply_config();
> if (apply_default_whitespace && parse_whitespace_option(state,
> apply_default_whitespace))
> - exit(1);
> + return -1;
> if (apply_default_ignorewhitespace &&
> parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
> - exit(1);
> + return -1;
> + return 0;
> }
>
> diff --git a/apply.h b/apply.h
> index 0f77f4d..f3b2ae4 100644
> --- a/apply.h
> +++ b/apply.h
> @@ -117,8 +117,8 @@ extern int parse_whitespace_option(struct apply_state
> *state,
> extern int parse_ignorewhitespace_option(struct apply_state *state,
> const char *option);
>
> -extern void init_apply_state(struct apply_state *state,
> -const char *prefix,
> -struct lock_file *lock_file);
> +extern int init_apply_state(struct apply_state *state,
> + const char *prefix,
> + struct lock_file *lock_file);
>
> #endif
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 805c707..b31f9eb 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -4724,7 +4724,8 @@ int cmd_apply(int argc, const char **argv, const char
> *prefix)
> OPT_END()
> };
>
> - init_apply_state(&state, prefix, NULL);
> + if (init_apply_state(&state, prefix, NULL))
> + exit(1);
>
> argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
> apply_usage, 0);
> --
> 2.8.2.490.g3dabe57
--
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 63/94] builtin/apply: make apply_all_patches() return -1 on error
On Wed, May 11, 2016 at 9:17 AM, Christian Couder
wrote:
> To finish libifying the apply functionality, apply_all_patches() should not
> die() or exit() in case of error, but return -1.
>
> While doing that we must take care that file descriptors are properly closed
> and, if needed, reset a sensible value.
>
> Signed-off-by: Christian Couder
> ---
> diff --git a/builtin/apply.c b/builtin/apply.c
> @@ -4613,9 +4613,10 @@ static int apply_all_patches(struct apply_state *state,
> }
>
> if (state->update_index) {
> - if (write_locked_index(&the_index, state->lock_file,
> COMMIT_LOCK))
> - die(_("Unable to write new index file"));
> + res = write_locked_index(&the_index, state->lock_file,
> COMMIT_LOCK);
> state->newfd = -1;
Does write_locked_index() unconditionally close the file descriptor
even when an error occurs? If not, then isn't this potentially leaking
'newfd'?
(My very cursory read of write_locked_index() seems to reveal that the
file descriptor may indeed remain open upon index write failure.)
> + if (res)
> + return error(_("Unable to write new index file"));
> }
>
> return !!errs;
--
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] attr.c: simplify macroexpand_one()
On Sun, May 15, 2016 at 8:31 PM, Eric Sunshine wrote: > On Sun, May 15, 2016 at 6:57 PM, Junio C Hamano wrote: >> The double-loop wants to do an early return immediately when one >> matching macro is found. Eliminate the extra variable 'a' used for >> that purpose and rewrite the "assign found itme to 'a' to make it > > What's "itme"? item. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit.c: use strchrnul() to scan for one line
Hi Junio,
On Sun, 15 May 2016, Junio C Hamano wrote:
> diff --git a/commit.c b/commit.c
> index 3f4f371..1f9ee8a 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -415,8 +415,7 @@ int find_commit_subject(const char *commit_buffer, const
> char **subject)
> p++;
> if (*p) {
> p += 2;
> - for (eol = p; *eol && *eol != '\n'; eol++)
> - ; /* do nothing */
> + eol = strchrnul(p, '\n');
> } else
> eol = p;
ACK. This was my fault, when I introduced the code in 9509af68 (Make
git-revert & git-cherry-pick a builtin, 2007-03-01). To be fair,
strchrnul() was introduced only later, in 659c69c (Add strchrnul(),
2007-11-09).
Ciao,
Dscho
--
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 0/2] Work on t3404 in preparation for rebase--helper
Hi Junio, On Fri, 13 May 2016, Junio C Hamano wrote: > Johannes Schindelin writes: > > > On Thu, 12 May 2016, Junio C Hamano wrote: > > > >> I took these separately already, and plan to fast-track them as they > >> are both "trivially correct"; I double checked that what I have match > >> these two, too. > > > > Oh, okay. I just wanted to make things easier for you, and now that I > > have a script to prepare patch series, it's really almost as trivial > > for me to send out a new iteration as it would be to update a Pull > > Request on GitHub. > > > > Do you want me to hold off with new iterations in the future until you > > clarified your preferred course of action? > > No. You've been doing great. I just wanted to clarify what I did > to your patch before I merge them separately to 'next' ahead of the > remainder that you'd be sending out, expecting a possible course > correction, e.g. "that would make it harder to queue the other patches > yet to come, all of which would depend on both of them--it would be > better to queue them on a single topic to be extended with these > other patches, after all these two are not that urgent". Thanks. I planned to work on the remainder as a "topic thicket" using my Git garden shears [*1*] anyway, picking up the changes you picked up, replacing my original patches. Ciao, Dscho Footnote *1*: The garden shears are kind of a `git rebase -i -p` as I wish I had designed it originally. Thet live here: https://github.com/git-for-windows/build-extra/blob/master/shears.sh -- 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] Ignore dirty submodule states during stash
On Sun, May 15, 2016 at 7:07 PM, Vasily Titskiy wrote:
> Do not save states of submodules as stash should ignore it.
Can you explain why this is a good idea?
(It is not obvious to me either way.)
Do we need a test/documentation updates for this?
>
> Signed-off-by: Vasily Titskiy
> ---
> git-stash.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index c7c65e2..b500c44 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -116,7 +116,7 @@ create_stash () {
> git read-tree --index-output="$TMPindex" -m $i_tree &&
> GIT_INDEX_FILE="$TMPindex" &&
> export GIT_INDEX_FILE &&
> - git diff --name-only -z HEAD -- >"$TMP-stagenames" &&
> + git diff --name-only --ignore-submodules -z HEAD --
> >"$TMP-stagenames" &&
> git update-index -z --add --remove --stdin
> <"$TMP-stagenames" &&
> git write-tree &&
> rm -f "$TMPindex"
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] bisect--helper: `bisect_voc` shell function in C
Hi Pranit, On Sat, 14 May 2016, Pranit Bauva wrote: > Reimplement the `bisect_voc` shell function in C. This is a too small > function to be called as a subcommand though the working of this > function has been tested by calling it as a subcommand. This leaves me puzzled as to what this patch is supposed to do. Maybe rename this function to have a more intuitive name, and then throw in a patch that makes use of the function? Ciao, Dscho -- 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
