* [PATCH 1/6] builtin/tag: move format specifier to global var
From: santiago @ 2016-09-22 18:53 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Santiago Torres
In-Reply-To: <20160922185317.349-1-santiago@nyu.edu>
From: Santiago Torres <santiago@nyu.edu>
The format specifier will be likely used in other functions throughout
git tag. One likely candidate to require format strings in the future is
the gpg_verify_tag function. However, changing the signature of
functions such as for_each_ref or verify_tag would be quite burdensome.
Instead, we move the format string specifier to a static global variable
that modules can access in the same way git-log and other modules handle
this case.
Signed-off-by: Santiago Torres <santiago@nyu.edu>
---
builtin/tag.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..dbf271f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -30,8 +30,9 @@ static const char * const git_tag_usage[] = {
static unsigned int colopts;
static int force_sign_annotate;
+static const char *fmt_pretty;
-static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)
+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)
{
struct ref_array array;
char *to_free = NULL;
@@ -42,23 +43,23 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
if (filter->lines == -1)
filter->lines = 0;
- if (!format) {
+ if (!fmt_pretty) {
if (filter->lines) {
to_free = xstrfmt("%s %%(contents:lines=%d)",
"%(align:15)%(refname:strip=2)%(end)",
filter->lines);
- format = to_free;
+ fmt_pretty = to_free;
} else
- format = "%(refname:strip=2)";
+ fmt_pretty = "%(refname:strip=2)";
}
- verify_ref_format(format);
+ verify_ref_format(fmt_pretty);
filter->with_commit_tag_algo = 1;
filter_refs(&array, filter, FILTER_REFS_TAGS);
ref_array_sort(sorting, &array);
for (i = 0; i < array.nr; i++)
- show_ref_array_item(array.items[i], format, 0);
+ show_ref_array_item(array.items[i], fmt_pretty, 0);
ref_array_clear(&array);
free(to_free);
@@ -334,7 +335,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct strbuf err = STRBUF_INIT;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
- const char *format = NULL;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -369,7 +369,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
N_("print only tags of the object"), 0, parse_opt_object_name
},
- OPT_STRING( 0 , "format", &format, N_("format"), N_("format to use for the output")),
+ OPT_STRING( 0 , "format", &fmt_pretty, N_("format"), N_("format to use for the output")),
OPT_END()
};
@@ -410,7 +410,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
run_column_filter(colopts, &copts);
}
filter.name_patterns = argv;
- ret = list_tags(&filter, sorting, format);
+ ret = list_tags(&filter, sorting);
if (column_active(colopts))
stop_column_filter();
return ret;
--
2.10.0
^ permalink raw reply related
* [PATCH 4/6] tag: add format specifier to gpg_verify_tag
From: santiago @ 2016-09-22 18:53 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Lukas P, Lukas Puehringer
In-Reply-To: <20160922185317.349-1-santiago@nyu.edu>
From: Lukas P <luk.puehringer@gmail.com>
Calling functions for gpg_verify_tag() may desire to print relevant
information about the header for further verification. Add an optional
format argument to print any desired information after GPG verification.
Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
---
builtin/tag.c | 2 +-
builtin/verify-tag.c | 6 ++++--
tag.c | 14 ++++++++++++--
tag.h | 4 ++--
4 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index dbf271f..94ed8a2 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -106,7 +106,7 @@ static int delete_tag(const char *name, const char *ref,
static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
{
- return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
+ return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
}
static int do_sign(struct strbuf *buffer)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148..7a1121b 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -51,8 +51,10 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
- else if (gpg_verify_tag(sha1, name, flags))
- had_error = 1;
+ else {
+ if (verify_and_format_tag(sha1, name, NULL, flags))
+ had_error = 1;
+ }
}
return had_error;
}
diff --git a/tag.c b/tag.c
index d1dcd18..e08da51 100644
--- a/tag.c
+++ b/tag.c
@@ -3,6 +3,7 @@
#include "commit.h"
#include "tree.h"
#include "blob.h"
+#include "ref-filter.h"
const char *tag_type = "tag";
@@ -30,8 +31,8 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
return ret;
}
-int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
- unsigned flags)
+int verify_and_format_tag(const unsigned char *sha1, const char *name_to_report,
+ const char *fmt_pretty, unsigned flags)
{
enum object_type type;
char *buf;
@@ -56,6 +57,15 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
ret = run_gpg_verify(buf, size, flags);
free(buf);
+
+ if (fmt_pretty) {
+ struct ref_array_item *ref_item;
+ ref_item = new_ref_item(name_to_report, sha1, 0);
+ ref_item->kind = FILTER_REFS_TAGS;
+ show_ref_item(ref_item, fmt_pretty, 0);
+ free_ref_item(ref_item);
+ }
+
return ret;
}
diff --git a/tag.h b/tag.h
index a5721b6..460b6f9 100644
--- a/tag.h
+++ b/tag.h
@@ -17,7 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void *data, unsigned long si
extern int parse_tag(struct tag *item);
extern struct object *deref_tag(struct object *, const char *, int);
extern struct object *deref_tag_noverify(struct object *);
-extern int gpg_verify_tag(const unsigned char *sha1,
- const char *name_to_report, unsigned flags);
+extern int verify_and_format_tag(const unsigned char *sha1,
+ const char *name_to_report, const char *fmt_pretty, unsigned flags);
#endif /* TAG_H */
--
2.10.0
^ permalink raw reply related
* [PATCH 2/6] gpg-interface: add GPG_VERIFY_QUIET flag
From: santiago @ 2016-09-22 18:53 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Lukas P, Lukas Puehringer
In-Reply-To: <20160922185317.349-1-santiago@nyu.edu>
From: Lukas P <luk.puehringer@gmail.com>
Functions that print git object information may require that the
gpg-interface functions be silent. Add a GPG_VERIFY_QUIET to prevent
functions such as `print_signature_buffer` from printing any output and
only return whether signature verification passed or not.
Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
---
gpg-interface.c | 3 +++
gpg-interface.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/gpg-interface.c b/gpg-interface.c
index 8672eda..b82bc50 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -88,6 +88,9 @@ int check_signature(const char *payload, size_t plen, const char *signature,
void print_signature_buffer(const struct signature_check *sigc, unsigned flags)
{
+ if (flags & GPG_VERIFY_QUIET)
+ return;
+
const char *output = flags & GPG_VERIFY_RAW ?
sigc->gpg_status : sigc->gpg_output;
diff --git a/gpg-interface.h b/gpg-interface.h
index ea68885..85dc982 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -3,6 +3,7 @@
#define GPG_VERIFY_VERBOSE 1
#define GPG_VERIFY_RAW 2
+#define GPG_VERIFY_QUIET 4
struct signature_check {
char *payload;
--
2.10.0
^ permalink raw reply related
* [PATCH 6/6] builtin/tag: add --format argument for tag -v
From: santiago @ 2016-09-22 18:53 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Lukas P, Lukas Puehringer
In-Reply-To: <20160922185317.349-1-santiago@nyu.edu>
From: Lukas P <luk.puehringer@gmail.com>
Adding --format to git tag -v mutes the default output of the GPG
verification and instead prints the formatted tag object.
This allows callers to cross-check the tagname from refs/tags with
the tagname from the tag object header upon GPG verification.
Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
---
builtin/tag.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 94ed8a2..3dd1e65 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
N_("git tag -d <tagname>..."),
N_("git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]"
"\n\t\t[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]"),
- N_("git tag -v <tagname>..."),
+ N_("git tag -v [--format=<format>] <tagname>..."),
NULL
};
@@ -106,7 +106,13 @@ static int delete_tag(const char *name, const char *ref,
static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
{
- return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
+ int flags;
+ flags = GPG_VERIFY_VERBOSE;
+
+ if (fmt_pretty)
+ flags = GPG_VERIFY_QUIET;
+
+ return verify_and_format_tag(sha1, name, fmt_pretty, flags);
}
static int do_sign(struct strbuf *buffer)
@@ -425,8 +431,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die(_("--merged and --no-merged option are only allowed with -l"));
if (cmdmode == 'd')
return for_each_tag_name(argv, delete_tag);
- if (cmdmode == 'v')
+ if (cmdmode == 'v') {
+ if (fmt_pretty)
+ verify_ref_format(fmt_pretty);
return for_each_tag_name(argv, verify_tag);
+ }
if (msg.given || msgfile) {
if (msg.given && msgfile)
--
2.10.0
^ permalink raw reply related
* [RFC/PATCH 0/6] Add --format to tag verification
From: santiago @ 2016-09-22 18:53 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Santiago Torres
From: Santiago Torres <santiago@nyu.edu>
Hello everyone,
This is a followup on [1]. There we discussed what would be the best way
to provide automated scripts with mechanisms to inspect the contents of
a tag upon verification.
We struggled a little bit with how to make this fit the current git
codebase in the best way. Specifically, we are not sure if adding the
GPG_QUIET flags and/or exposing the primitives to allocate individual
git_ref_item's would be the best way forward.
This applies on the current HEAD as well as v 2.9.10.
Thanks!
-Santiago.
P.S. Gmane seems to be broken for git after it was rebooted. Should we ping
them about it?
[1] http://lists-archives.com/git/869122-verify-tag-add-check-name-flag.html
Lukas P (4):
gpg-interface: add GPG_VERIFY_QUIET flag
ref-filter: Expose wrappers for ref_item functions
tag: add format specifier to gpg_verify_tag
builtin/tag: add --format argument for tag -v
Santiago Torres (2):
builtin/tag: move format specifier to global var
builtin/verify-tag: Add --format to verify-tag
builtin/tag.c | 33 +++++++++++++++++++++------------
builtin/verify-tag.c | 17 ++++++++++++++---
gpg-interface.c | 3 +++
gpg-interface.h | 1 +
ref-filter.c | 20 ++++++++++++++++++++
ref-filter.h | 10 ++++++++++
tag.c | 14 ++++++++++++--
tag.h | 4 ++--
8 files changed, 83 insertions(+), 19 deletions(-)
--
2.10.0
^ permalink raw reply
* [PATCH 3/6] ref-filter: Expose wrappers for ref_item functions
From: santiago @ 2016-09-22 18:53 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Lukas P, Lukas Puehringer
In-Reply-To: <20160922185317.349-1-santiago@nyu.edu>
From: Lukas P <luk.puehringer@gmail.com>
Ref-filter functions are useful for printing git object information
without a format specifier. However, some functions may not want to use
a complete ref-array, and just a single item instead. Expose
create/show/free functions for ref_array_items through wrappers around
the original functions.
Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
---
ref-filter.c | 20 ++++++++++++++++++++
ref-filter.h | 10 ++++++++++
2 files changed, 30 insertions(+)
diff --git a/ref-filter.c b/ref-filter.c
index 9adbb8a..b013799 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1329,6 +1329,14 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
return ref;
}
+/* Wrapper: Create ref_array_item w/o referencing container in function name */
+struct ref_array_item *new_ref_item(const char *refname,
+ const unsigned char *objectname,
+ int flag)
+{
+ return new_ref_array_item(refname, objectname, flag);
+}
+
static int filter_ref_kind(struct ref_filter *filter, const char *refname)
{
unsigned int i;
@@ -1426,6 +1434,12 @@ static void free_array_item(struct ref_array_item *item)
free(item);
}
+/* Wrapper: Free ref_array_item w/o referencing container in function name */
+void free_ref_item(struct ref_array_item *ref_item)
+{
+ free_array_item(ref_item);
+}
+
/* Free all memory allocated for ref_array */
void ref_array_clear(struct ref_array *array)
{
@@ -1637,6 +1651,12 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
putchar('\n');
}
+/* Wrapper: Show ref_array_item w/o referencing container in function name */
+void show_ref_item(struct ref_array_item *ref_item, const char *format, int quote_style)
+{
+ show_ref_array_item(ref_item, format, quote_style);
+}
+
/* If no sorting option is given, use refname to sort as default */
struct ref_sorting *ref_default_sorting(void)
{
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..0f0ffe9 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -107,4 +107,14 @@ struct ref_sorting *ref_default_sorting(void);
/* Function to parse --merged and --no-merged options */
int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset);
+/*
+ * Wrappers exposing the ref_array_item data structure independently
+ * of the container ref_array, e.g. to format-print individual refs.
+ */
+struct ref_array_item *new_ref_item(const char *refname,
+ const unsigned char *objectname, int flag);
+void show_ref_item(struct ref_array_item *ref_item, const char *format,
+ int quote_style);
+void free_ref_item(struct ref_array_item *ref_item);
+
#endif /* REF_FILTER_H */
--
2.10.0
^ permalink raw reply related
* Fwd: [git/git-scm.com] Git Gui crash on Mac OS Sierra (#853)
From: Přemek Koch @ 2016-09-22 18:44 UTC (permalink / raw)
To: git
In-Reply-To: <3DDE880D-8746-4E37-903E-B23B1AD338DC@gmail.com>
> Hello, maybe right place this time...
>
>
> I want to report a bug:
>
> On MacOS Sierra git gui crashes randomly with this message:
>
> 2016-09-22 13:17:36.759 Wish[23615:1501726] *** Terminating app due to uncaught exception 'CALayerInvalidGeometry', reason: 'CALayer position contains NaN: [0 nan]'
> *** First throw call stack:
> (
> 0 CoreFoundation 0x00007fff7f0407bb __exceptionPreprocess + 171
> 1 libobjc.A.dylib 0x00007fff937ada2a objc_exception_throw + 48
> 2 CoreFoundation 0x00007fff7f0bda65 +[NSException raise:format:] + 197
> 3 QuartzCore 0x00007fff84c09980 _ZN2CA5Layer12set_positionERKNS_4Vec2IdEEb + 152
> 4 QuartzCore 0x00007fff84c09af5 -[CALayer setPosition:] + 44
> 5 QuartzCore 0x00007fff84c0a14b -[CALayer setFrame:] + 644
> 6 CoreUI 0x00007fff8a9b0112 _ZN20CUICoreThemeRenderer26MakeOrUpdateScrollBarLayerEPK13CUIDescriptoraPP7CALayer + 1284
> 7 CoreUI 0x00007fff8a9ac317 _ZN20CUICoreThemeRenderer19CreateOrUpdateLayerEPK13CUIDescriptorPP7CALayer + 1755
> 8 CoreUI 0x00007fff8a92e4d1 _ZN11CUIRenderer19CreateOrUpdateLayerEPK14__CFDictionaryPP7CALayer + 175
> 9 CoreUI 0x00007fff8a931185 CUICreateOrUpdateLayer + 221
> 10 AppKit 0x00007fff7d675623 -[NSCompositeAppearance _callCoreUIWithBlock:options:] + 226
> 11 AppKit 0x00007fff7cd22a9d -[NSAppearance _createOrUpdateLayer:options:] + 76
> 12 AppKit 0x00007fff7cf9b143 -[NSScrollerImp _animateToRolloverState] + 274
> 13 AppKit 0x00007fff7cf5ab79 __49-[NSScrollerImp _installDelayedRolloverAnimation]_block_invoke + 673
> 14 AppKit 0x00007fff7ce21331 -[NSScrollerImp _doWork:] + 15
> 15 Foundation 0x00007fff80a3ec88 __NSFireDelayedPerform + 417
> 16 CoreFoundation 0x00007fff7efc0f44 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
> 17 CoreFoundation 0x00007fff7efc0bd3 __CFRunLoopDoTimer + 1075
> 18 CoreFoundation 0x00007fff7efc072a __CFRunLoopDoTimers + 298
> 19 CoreFoundation 0x00007fff7efb82f1 __CFRunLoopRun + 2081
> 20 CoreFoundation 0x00007fff7efb7874 CFRunLoopRunSpecific + 420
> 21 HIToolbox 0x00007fff7e557f6c RunCurrentEventLoopInMode + 240
> 22 HIToolbox 0x00007fff7e557ca9 ReceiveNextEventCommon + 184
> 23 HIToolbox 0x00007fff7e557bd6 _BlockUntilNextEventMatchingListInModeWithFilter + 71
> 24 AppKit 0x00007fff7cc4e5f5 _DPSNextEvent + 1093
> 25 AppKit 0x00007fff7d35e8eb -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1637
> 26 Tk 0x00000001047cc285 TkGenerateButtonEvent + 494
> 27 Tk 0x00000001047cc54d Tk_MacOSXSetupTkNotifier + 395
> 28 Tcl 0x00000001048be5a8 Tcl_DoOneEvent + 237
> 29 Tk 0x0000000104726f4f Tk_MainLoop + 33
> 30 Tk 0x0000000104732a5b Tk_MainEx + 1566
> 31 Wish 0x000000010470d55a Wish + 9562
> 32 libdyld.dylib 0x00007fff94089255 start + 1
> )
> libc++abi.dylib: terminating with uncaught exception of type NSException
> error: git-gui died of signal 6
>
>
> Premek Koch
> premek.koch@gmail.com
>
>
>
>> Začátek přeposílané zprávy:
>>
>> Od: Jean-Noël Avila <notifications@github.com>
>> Předmět: Re: [git/git-scm.com] Git Gui crash on Mac OS Sierra (#853)
>> Datum: 22. září 2016 19:43:30 SELČ
>> Komu: "git/git-scm.com" <git-scm.com@noreply.github.com>
>> Kopie: Premek Koch <premek.koch@gmail.com>, Author <author@noreply.github.com>
>> Odpověď na: "git/git-scm.com" <reply+000da9c18ad6de0541ea9352a4e1e834fbf1dde22ff10e3b92cf0000000113fbda4292a169ce0aa4f922@reply.github.com>
>>
>> This issue tracker is dedicated to the git-scm.com website.
>>
>> The issue you have raised is related to the git program. If you think there is a bug in the git program, please, send your issue to the git mailing list git@vger.kernel.org
>>
>> —
>> You are receiving this because you authored the thread.
>> Reply to this email directly, view it on GitHub, or mute the thread.
>>
>
^ permalink raw reply
* Re: .gitignore does not ignore Makefile
From: Timur Tabi @ 2016-09-22 18:44 UTC (permalink / raw)
To: Junio C Hamano, Kevin Daudt; +Cc: git
In-Reply-To: <xmqqy42j4wp9.fsf@gitster.mtv.corp.google.com>
Junio C Hamano wrote:
> It actually is even worse. As the user promised Git that the <file>
> will not be modified and will be kept the same as the version in the
> index, Git reserves the right to_overwrite_ it with the version in
> the index anytime when it is convenient to do so, removing whatever
> local change the user had despite the promise to Git. The "abort
> saying that the file has changed" is merely various codepaths in the
> current implementation trying to be extra nice.
So .gitignore only ignores new files, not modified ones? That seems
odd, but I guess that's the way it's always been and I just haven't
noticed until now.
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH v4 2/3] regex: add regexec_buf() that can work on a non NUL-terminated string
From: Johannes Schindelin @ 2016-09-22 18:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Benjamin Kramer, René Scharfe
In-Reply-To: <xmqqtwd96p0b.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Wed, 21 Sep 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > ...
> > Happily, there is an extension to regexec() introduced by the NetBSD
> > project and present in all major regex implementation including
> > Linux', MacOSX' and the one Git includes in compat/regex/: by using
> > the (non-POSIX) REG_STARTEND flag, it is possible to tell the
> > regexec() function that it should only look at the offsets between
> > pmatch[0].rm_so and pmatch[0].rm_eo.
> >
> > That is exactly what we need.
>
> Wonderful.
>
> > Since support for REG_STARTEND is so widespread by now, let's just
> > introduce a helper function that uses it, and fall back to allocating
> > and constructing a NUL-terminated when REG_STARTEND is not available.
>
> I'd somehow reword the last paragraph here, though ;-)
Oh drats. I thought I had prepared the fixed commit messages already :-(
What you have in `pu` as of today looks good.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] fetch-pack: do not reset in_vain on non-novel acks
From: Jonathan Tan @ 2016-09-22 18:36 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1474568670.git.jonathantanmy@google.com>
The MAX_IN_VAIN mechanism was introduced in commit f061e5f ("fetch-pack:
give up after getting too many "ack continue"", 2006-05-24) to stop ref
negotiation if a number of consecutive "have"s have been sent with no
corresponding new acks. A use case (as described in that commit) is the
scenario in which the local repository has more roots than the remote
repository.
However, during a negotiation in which stateless RPCs are used,
MAX_IN_VAIN will (almost) never trigger (in the more-roots scenario
above and others) because in each new request, the client has to inform
the server of objects it already has and knows the server has (to remind
the server of the state), which the server then acks.
Make fetch-pack only consider novel acks (acks for objects for which the
client has never received an ack before in this session) as new acks for
the purpose of MAX_IN_VAIN.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
fetch-pack.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 85e77af..1141e3c 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -428,10 +428,18 @@ static int find_common(struct fetch_pack_args *args,
const char *hex = sha1_to_hex(result_sha1);
packet_buf_write(&req_buf, "have %s\n", hex);
state_len = req_buf.len;
- }
+ /*
+ * Reset in_vain because this
+ * ack is a novel ack (that is,
+ * an ack for this commit has
+ * not been seen).
+ */
+ in_vain = 0;
+ } else if (!args->stateless_rpc
+ || ack != ACK_common)
+ in_vain = 0;
mark_common(commit, 0, 1);
retval = 0;
- in_vain = 0;
got_continue = 1;
if (ack == ACK_ready) {
clear_prio_queue(&rev_list);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH] do not reset in_vain on non-novel acks
From: Jonathan Tan @ 2016-09-22 18:36 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
This is regarding the packfile negotiation in fetch-pack. If there is a
concern that MAX_IN_VAIN would be hit too early (as a consequence of the
patch below), I'm currently investigating the possibility of improving
the negotiation ability of the client side further (for example, by
prioritizing refs or heads instead of merely prioritizing by date in the
priority queue of objects), but I thought I'd send the patch out first
anyway to see what others think.
Jonathan Tan (1):
fetch-pack: do not reset in_vain on non-novel acks
fetch-pack.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply
* Re: .gitignore does not ignore Makefile
From: Junio C Hamano @ 2016-09-22 18:26 UTC (permalink / raw)
To: Kevin Daudt; +Cc: Timur Tabi, git
In-Reply-To: <20160922154421.GA6641@ikke.info>
Kevin Daudt <me@ikke.info> writes:
> Often people advise tricks like `git update-index --assume-unchanges
> <file>`, but this does not work as expected. It's merely a promise to
> git that this file does not change (and hence, git will not check if
> this file has changed when doing git status), but command that try to
> change this file will abort saying that the file has changed.
It actually is even worse. As the user promised Git that the <file>
will not be modified and will be kept the same as the version in the
index, Git reserves the right to _overwrite_ it with the version in
the index anytime when it is convenient to do so, removing whatever
local change the user had despite the promise to Git. The "abort
saying that the file has changed" is merely various codepaths in the
current implementation trying to be extra nice.
^ permalink raw reply
* Re: [PATCH] verify_packfile: check pack validity before accessing data
From: Junio C Hamano @ 2016-09-22 18:21 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20160922040523.x3pihbs7f5iudyfz@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So I wanted to know whether there were any code paths that failed to do
> so, and just blindly rely on the lazy-open. Finding the races is
> inherently hard, because you only catch them when somebody else is doing
> a repack. But if we just _remove_ the lazy-load, then it becomes easy to
> catch anybody relying on it. Like:
> ...
Clever; I like it.
> In such a case, we are relying on the lazy-load (and we _are_ racy!).
> But the patch above would punish people on low-descriptor systems. It's
> better to have an unlikely race and complete the request than to fail
> consistently. :-/
>
> For people who are running high-traffic servers, they just need to make
> sure their file descriptor limit is reasonably high to avoid the race.
Thanks for an illuminating backstory for the patch.
^ permalink raw reply
* Re: [PATCH 1/2] ls-files: adding support for submodules
From: Junio C Hamano @ 2016-09-22 18:13 UTC (permalink / raw)
To: Jeff King; +Cc: Brandon Williams, git
In-Reply-To: <20160922041854.7754ujcynhk7mdnh@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Should this option just be "--prefix", or maybe "--output-prefix"?
> Submodules are the obvious use case here, but I could see somebody
> adapting this for other uses (alternatively, if we _do_ want to keep it
> just as an implementation detail for submodules, we should probably
> discourage people in the documentation from using it themselves).
I agree that this is not specific to submodules; this is closely
related to what we internally call "prefix", but is different.
In any case, I would strongly recommend against exposing this (or
anything for that matter) "--prefix" to the end-user, especially
because this feature is likely to be applicable to many subcommands,
and some subcommands would want different sort of prefixing made to
different things. Think of "git diff" that has a way to customize
the "a/" and "b/" part in "git --diff a/$path b/$path", and has
learned another way to prepend an additional prefix to every line of
output via "--line-prefix". We want to give reasonably specific
names to things so that readers can tell what it affects from the
name of an option.
What we internally call "prefix" and "--submodule-prefix" is closely
related in that they both interact with pathspecs. "prefix" gets
prepended to elements of an end-user supplied pathspec before a
full-path-in-the-repository (i.e. a path in the index and a path
relative from the top of the working tree) is matched against them.
This new thing on the other hand allows the leading part of pathspec
elements to be above the full-path-in-the-repository. For example,
my primary Git working area is at ~/w/git.git and I could say
$ cd ~
$ git -C w/git.git/ ls-files \
--submodule-prefix=w/git.git -- 'w/git.git/D\*' |
xargs ls -1 -l
The command starts (eh, at least "pretends to start") in the top of
my working tree, which means that the "prefix" is NULL. But the
shell that spawns the "git" command is still sitting in my home
directory, and viewed from there, the Documentation subdirectory is
at w/git.git/Documentation/, which is what I gave the command as the
sole element of the pathspec. From "ls-files"'s point of view, each
path it discovers in the index (and in the working tree) is first
prefixed with --submodule-prefix before it gets matched against this
pathspec and the result is shown with this prefix, so that the
output gets relative to the calling shell and xargs would find them
at expected places. The --submodule-prefix acts to cancel out the
fact that the caller sits a few levels above the working tree and
gave a pathspec with elements relative to that higher level in the
directory hierarchy.
Obviously, the above example is *not* using submodules at all, and
demonstrates that the mechanism does not have to be used solely to
implement recurse-into-submodules behaviour, so in that sense, the
name of the option is not quite accurate. It however is a good
demonstration why it is *not* "output prefix". It makes me wonder
if pathspec-prefix is a better name, but that may give an incorrect
impression that this would be used only for matching and would not
affect the output. Do we need separate options to specify what gets
prefixed to the in-repository path before paths are matched against
a pathspec (i.e. "--pathspec-prefix") and what gets prefixed to the
resulting paths in the output (i.e. "--output-path-prefix")?
A few further random thoughts.
* As Stefan alluded to (much) earlier, it might be a better idea
to have these 'prefix' as the global option to "git" potty, not
to each subcommand that happens to support them;
* It is unclear how this should interact with commands that are run
in a subdirectory of the working tree. E.g. what should the
prefix and the pathspec look like if the command in the above
example is started in w/git.git/Documentation subdirectory, i.e.
$ cd ~
$ git -C w/git.git/Documentation ls-files \
--submodule-prefix=??????? -- '???????' |
xargs ls -1 -l
Should we error out if we are not at the top of the working tree
when --submodule-prefix is given?
* It is further unclear how this should interact with commands that
can give "relative" paths output if we allowed them to start from
a subdirectory of the working tree. E.g. "git grep" gives its
output relative to where it was started from:
$ cd ~/w/git.git/Documentation
$ git grep -c 'ERROR.*frotz' -- '*.txt'
git-checkout.txt:1
How should it interact with the --submodule-prefix option and
what value should the caller give to the option and how should
the caller adjust the pathspec if it wants to get paths relative
to my home directory? I do not think the following is correct,
but I am not sure what the correct values of them should be:
$ cd ~
$ git -C w/git.git/Documentation \
--submodule-prefix=w/git.git/ \
grep -c 'ERROR.*frotz' -- 'w/git.git/Documentation/*.txt'
This becomes a non-issue if we forbid use of this new prefix when
"git" is not started at the top of the working tree.
^ permalink raw reply
* Re: [PATCH v2 2/3] init: do not set core.worktree more often than necessary
From: Junio C Hamano @ 2016-09-22 17:27 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Michael J Gruber, Max Nordlund
In-Reply-To: <CACsJy8BQvo_XSXZ3m8=A=0tik3qPd8k9cyC4G408JK+AhtYStQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
>> Perhaps a comment before init_db() to tell callers to always call
>> the other one is the least thing necessary?
>
> Good thinking. We could go a step further, baking it as assert() to
> catch new/incorrect call sequences automatically.
>
> Or we could combine the two functions init_db() and set_git_dir_init()
> into one. I prefer this one, but having problem with finding a good
> name for it ...
It probably is a reasonable way forward to name the combined
function that takes the parameters that are taken by both functions
in their current incarnation init_db(). After all, initializing the
Git repository database involves telling the function where the .git
directory is, among other things like where to take the template for
a newly created repository is.
^ permalink raw reply
* Re: Bug: pager.<cmd> doesn't work well with editors
From: Junio C Hamano @ 2016-09-22 17:19 UTC (permalink / raw)
To: Jeff King; +Cc: Anatoly Borodin, git
In-Reply-To: <20160922064730.277nzkqlxbcx2kjg@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I don't think it is a bad move overall. I use "pager.log" to pipe
> through a specific command (that is different than I would use for other
> commands).
>
> So I like the idea of configurability; the problem is just that it is
> happening at the wrong level.
The level at which configurability happens might be one issue
(i.e. you may want different pager for two operating modes for the
same command, hence your need to use "tag.list" not just "tag"), but
I think another issue is that it conflates if the output need to be
paged (on/off) and what pager should be used when the output is
paged. When we see that a user sets "pager.tag", we should not have
made it an instruction to Git that _all_ output from "git tag" must
be paged.
If there were no need for supporting separate pagers per operating
mode of a Git command, say "git tag", you would not want to page the
output unless you are producing "git tag [-l]" listing. You do not
want your interaction with the usual "git tag <name> [<an object>]"
to be paged, even if you want to use a pager different from GIT_PAGER
when you are viewing the tags.
It is good that each codepath can give default in this example
> The individual commands should be in
> charge of it, with something like:
>
> setup_auto_pager("log", 1);
> ...
> if (mode_list)
> setup_auto_pager("tag.list", 0);
as the second parameter to setup_auto_pager(), but I think the first
parameter being "tag.list" vs "tag" is a separate issue. Until
there comes another codepath in "git tag" that wants to call
setup_auto_pager(), it does not make any difference from the end-user's
point of view. Starting with "tag.list" may futureproof it
(e.g. perhaps somebody wants to use a separate pager for "git tag --help"
and "tag.help" can be added without disrupting existing use of "tag.list")
So I think we are fundamentally on the same page; it is just you are
aiming higher than I was, but we both recognize the need for separate
codepaths in a single command to decide if the output should be paged.
> I don't have a particular plan to work on it anytime soon, but maybe
> somebody could pick it up as relatively low-hanging fruit.
;-)
^ permalink raw reply
* [PATCH] run-command: async_exit no longer needs to be public
From: Ramsay Jones @ 2016-09-22 16:56 UTC (permalink / raw)
To: Lars Schneider; +Cc: Junio C Hamano, Jeff King, GIT Mailing-list
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
Hi Lars,
If you need to re-roll your 'ls/filter-process' branch, could you please
squash this into the relevant commit c42a4cbc ("run-command: move check_pipe()
from write_or_die to run_command", 20-09-2016).
[Note that commit 9658846c ("write_or_die: handle EPIPE in async threads",
24-02-2016) introduced async_exit() specifically for use in the implementation
of check_pipe(). Now that you have moved check_pipe() into run-command.c,
it no longer needs to be public.]
Thanks!
ATB,
Ramsay Jones
run-command.c | 30 +++++++++++++++---------------
run-command.h | 3 +--
2 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/run-command.c b/run-command.c
index b72f6d1..3269362 100644
--- a/run-command.c
+++ b/run-command.c
@@ -6,19 +6,6 @@
#include "thread-utils.h"
#include "strbuf.h"
-void check_pipe(int err)
-{
- if (err == EPIPE) {
- if (in_async())
- async_exit(141);
-
- signal(SIGPIPE, SIG_DFL);
- raise(SIGPIPE);
- /* Should never happen, but just in case... */
- exit(141);
- }
-}
-
void child_process_init(struct child_process *child)
{
memset(child, 0, sizeof(*child));
@@ -647,7 +634,7 @@ int in_async(void)
return !pthread_equal(main_thread, pthread_self());
}
-void NORETURN async_exit(int code)
+static void NORETURN async_exit(int code)
{
pthread_exit((void *)(intptr_t)code);
}
@@ -697,13 +684,26 @@ int in_async(void)
return process_is_async;
}
-void NORETURN async_exit(int code)
+static void NORETURN async_exit(int code)
{
exit(code);
}
#endif
+void check_pipe(int err)
+{
+ if (err == EPIPE) {
+ if (in_async())
+ async_exit(141);
+
+ signal(SIGPIPE, SIG_DFL);
+ raise(SIGPIPE);
+ /* Should never happen, but just in case... */
+ exit(141);
+ }
+}
+
int start_async(struct async *async)
{
int need_in, need_out;
diff --git a/run-command.h b/run-command.h
index e7c5f71..bb89c30 100644
--- a/run-command.h
+++ b/run-command.h
@@ -54,7 +54,6 @@ int finish_command(struct child_process *);
int finish_command_in_signal(struct child_process *);
int run_command(struct child_process *);
-void check_pipe(int err);
/*
* Returns the path to the hook file, or NULL if the hook is missing
@@ -141,7 +140,7 @@ struct async {
int start_async(struct async *async);
int finish_async(struct async *async);
int in_async(void);
-void NORETURN async_exit(int code);
+void check_pipe(int err);
/**
* This callback should initialize the child process and preload the
--
2.10.0
^ permalink raw reply related
* [PATCH] introduce CHECKOUT_INIT
From: René Scharfe @ 2016-09-22 16:11 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
Add a static initializer for struct checkout and use it throughout the
code base. It's shorter, avoids a memset(3) call and makes sure the
base_dir member is initialized to a valid (empty) string.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
apply.c | 4 +---
builtin/checkout-index.c | 2 +-
builtin/checkout.c | 3 +--
cache.h | 1 +
unpack-trees.c | 4 +---
5 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/apply.c b/apply.c
index e327021..b03d274 100644
--- a/apply.c
+++ b/apply.c
@@ -3334,10 +3334,8 @@ static void prepare_fn_table(struct apply_state *state, struct patch *patch)
static int checkout_target(struct index_state *istate,
struct cache_entry *ce, struct stat *st)
{
- struct checkout costate;
+ struct checkout costate = CHECKOUT_INIT;
- memset(&costate, 0, sizeof(costate));
- costate.base_dir = "";
costate.refresh_cache = 1;
costate.istate = istate;
if (checkout_entry(ce, &costate, NULL) || lstat(ce->name, st))
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 92c6967..30a49d9 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -16,7 +16,7 @@ static int checkout_stage; /* default to checkout stage0 */
static int to_tempfile;
static char topath[4][TEMPORARY_FILENAME_LENGTH + 1];
-static struct checkout state;
+static struct checkout state = CHECKOUT_INIT;
static void write_tempfile_record(const char *name, const char *prefix)
{
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 9941abc..4c86272 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -239,7 +239,7 @@ static int checkout_paths(const struct checkout_opts *opts,
const char *revision)
{
int pos;
- struct checkout state;
+ struct checkout state = CHECKOUT_INIT;
static char *ps_matched;
struct object_id rev;
struct commit *head;
@@ -352,7 +352,6 @@ static int checkout_paths(const struct checkout_opts *opts,
return 1;
/* Now we are committed to check them out */
- memset(&state, 0, sizeof(state));
state.force = 1;
state.refresh_cache = 1;
state.istate = &the_index;
diff --git a/cache.h b/cache.h
index d0494c8..5d9116c 100644
--- a/cache.h
+++ b/cache.h
@@ -1354,6 +1354,7 @@ struct checkout {
not_new:1,
refresh_cache:1;
};
+#define CHECKOUT_INIT { NULL, "" }
#define TEMPORARY_FILENAME_LENGTH 25
extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);
diff --git a/unpack-trees.c b/unpack-trees.c
index 3db3f02..ea6bdd2 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1094,12 +1094,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
int i, ret;
static struct cache_entry *dfc;
struct exclude_list el;
- struct checkout state;
+ struct checkout state = CHECKOUT_INIT;
if (len > MAX_UNPACK_TREES)
die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
- memset(&state, 0, sizeof(state));
- state.base_dir = "";
state.force = 1;
state.quiet = 1;
state.refresh_cache = 1;
--
2.10.0
^ permalink raw reply related
* Re: [PATCH 1/2] ls-files: adding support for submodules
From: Stefan Beller @ 2016-09-22 16:04 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Brandon Williams, git@vger.kernel.org
In-Reply-To: <20160922041854.7754ujcynhk7mdnh@sigill.intra.peff.net>
On Wed, Sep 21, 2016 at 9:18 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Sep 21, 2016 at 04:13:22PM -0700, Junio C Hamano wrote:
>
>> Brandon Williams <bmwill@google.com> writes:
>>
>> > yes you mentioned this and I meant to change that before sending it out.
>> > Looks like it slipped through have slipped through.
>>
>> I already fixed it up locally when I sent the reply, but thanks for
>> resending (which assures me that your local copy is up-to-date and I
>> do not have to worry about having to repeat me in the future, if
>> this ever needs further rerolling ;-).
>
> While we are on the subject, the commit message also uses some past
> tense:
>
> Allow ls-files to recognize submodules in order to retrieve a list of
> files from a repository's submodules. This is done by forking off a
> process to recursively call ls-files on all submodules. Also added a
> submodule-prefix command in order to prepend paths to child processes.
>
> The final sentence should be "Also add...".
>
> Since this final bit of logic was sufficiently non-obvious that it only
> came about in v2, maybe it is worth describing a little more fully:
>
> Also add a submodule-prefix option, which instructs the child
> processes to prepend the prefix to each path they output. This makes
> the output paths match what is on the filesystem (i.e., as if the
> submodule boundaries were not there at all).
>
> Should this option just be "--prefix", or maybe "--output-prefix"?
I think --prefix can easily be confused with the internal prefix that we hand
down to each command. --output-prefix works for the ls-files case, but as
soon as we do more than just printing out file names, we'd use that prefix for
more than just output, e.g. in grep it becomes part of the pathspec IIUC.
I agree however that we probably don't want to keep it submodule specific.
> Submodules are the obvious use case here, but I could see somebody
> adapting this for other uses (alternatively, if we _do_ want to keep it
> just as an implementation detail for submodules, we should probably
> discourage people in the documentation from using it themselves).
>
> -Peff
^ permalink raw reply
* Re: Re: Re: Homebrew and Git
From: Stefan Beller @ 2016-09-22 15:57 UTC (permalink / raw)
To: Jonas Thiel; +Cc: Jeff King, John Keeping, Heiko Voigt, git@vger.kernel.org
In-Reply-To: <trinity-7a55c197-21af-4808-9919-6fc26bdcece2-1474536212197@3capp-gmx-bs75>
On Thu, Sep 22, 2016 at 2:23 AM, Jonas Thiel <jonas.lierschied@gmx.de> wrote:
> Sorry for my late reply. Thanks for your support -- I really appreciate that.
>
> @Jeff: Unfortunately, I do not know how to implement the patch you provided. Can you explain how to do that?
I think this should do:
git clone https://github.com/git/git
cd git
# get the email
wget http://public-inbox.org/git/20160921084841.phq7cfbagi5k7ku4@sigill.intra.peff.net/raw
# apply patch:
git am raw
make
make install
>
> Thanks and best regards,
> Jonas
>
>> Gesendet: Mittwoch, 21. September 2016 um 10:48 Uhr
>> Von: "Jeff King" <peff@peff.net>
>> An: "John Keeping" <john@keeping.me.uk>
>> Cc: "Heiko Voigt" <hvoigt@hvoigt.net>, "Jonas Thiel" <jonas.lierschied@gmx.de>, git@vger.kernel.org
>> Betreff: Re: Re: Homebrew and Git
>>
>> On Tue, Sep 20, 2016 at 08:15:55PM +0100, John Keeping wrote:
>>
>> > > BTW, here is the callstack inlined from the crashreport:
>> > >
>> > > bsystem_platform.dylib 0x00007fff840db41c _platform_strchr$VARIANT$Haswell + 28
>> > > 1 git 0x000000010ba1d3f4 ident_default_email + 801
>> > > 2 git 0x000000010ba1d68f fmt_ident + 66
>> > > 3 git 0x000000010ba4b495 files_log_ref_write + 175
>> > > 4 git 0x000000010ba4b0a6 commit_ref_update + 106
>> > > 5 git 0x000000010ba4c3a8 ref_transaction_commit + 468
>> > > 6 git 0x000000010b994dd8 s_update_ref + 271
>> > > 7 git 0x000000010b994556 fetch_refs + 1969
>> > > 8 git 0x000000010b9935f2 fetch_one + 1913
>> > > 9 git 0x000000010b992bc4 cmd_fetch + 549
>> > > 10 git 0x000000010b9666c4 handle_builtin + 478
>> > > 11 git 0x000000010b96602f main + 376
>> > > 12 libdyld.dylib 0x00007fff834ef5ad start + 1
>> > >
>> > > Maybe someone else has an idea what might be causing this...
>> >
>> > The only strchr I can see that could be called here is in
>> > canonical_name(), where it's called with addrinfo::ai_canonname.
>>
>> There's one in add_domainname(), too, but it can never be NULL (we could
>> walk off the end of the buffer, but only if gethostname() lies to us
>> about its result code, which seems unlikely). So I agree it's probably
>> the call in canonical_name().
>>
>> > Searching for OS X and ai_canonname, leads me straight back to this
>> > list, although 7 years ago! I think ident.c needs a fix similar to
>> > commit 3e8a00a (daemon.c: fix segfault on OS X, 2009-04-27); from the
>> > commit message there:
>> >
>> > On OS X (and maybe other unices), getaddrinfo(3) returns NULL
>> > in the ai_canonname field if it's called with an IP address for
>> > the hostname.
>>
>> Interesting. We are already prepared for failure from getaddrinfo()
>> here, so probably:
>>
>> diff --git a/ident.c b/ident.c
>> index e20a772..d17b5bd 100644
>> --- a/ident.c
>> +++ b/ident.c
>> @@ -101,7 +101,7 @@ static int canonical_name(const char *host, struct strbuf *out)
>> memset (&hints, '\0', sizeof (hints));
>> hints.ai_flags = AI_CANONNAME;
>> if (!getaddrinfo(host, NULL, &hints, &ai)) {
>> - if (ai && strchr(ai->ai_canonname, '.')) {
>> + if (ai && ai->ai_canonname && strchr(ai->ai_canonname, '.')) {
>> strbuf_addstr(out, ai->ai_canonname);
>> status = 0;
>> }
>>
>> would be sufficient. Jonas, can you see if that patch helps?
>>
>> -Peff
>>
^ permalink raw reply
* Re: .gitignore does not ignore Makefile
From: Kevin Daudt @ 2016-09-22 15:44 UTC (permalink / raw)
To: Timur Tabi; +Cc: git
In-Reply-To: <CAOZdJXWpcSZ+jAoV8HttkaB7Fh=wzWDTCsHy8W-S9xOOBodVFw@mail.gmail.com>
On Thu, Sep 22, 2016 at 09:19:22AM -0500, Timur Tabi wrote:
> I have the following .gitignore file in patch arm/arm64/boot/dts:
>
> *.dtb
> qcom
> qcom.orig
>
> When I do a git status, I see this:
>
> modified: .gitignore
> modified: qcom/Makefile
>
> All of the other files in arm/arm64/boot/dts/qcom are being ignored,
> as request. However, the file "Makefile" is not being ignored. Why?
> What's so special about "Makefile" that git refuses to ignore it?
>
There is nothing special about the Makefile, except that it's tracked.
Git never ignores tracked files (almost a paradox).
You can untrack the file by doing git rm --cached <file>, which new
commits won't have this file.
If your goal is to ignore local changes to a tracked file, then the
advise is to reconsider your plan.
Often people advise tricks like `git update-index --assume-unchanges
<file>`, but this does not work as expected. It's merely a promise to
git that this file does not change (and hence, git will not check if
this file has changed when doing git status), but command that try to
change this file will abort saying that the file has changed.
Hope this helps,
Kevin.
^ permalink raw reply
* Re: [PATCH] clone: pass --progress decision to recursive submodules
From: Stefan Beller @ 2016-09-22 15:36 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20160922052446.iwr62hpa2meal7uj@sigill.intra.peff.net>
On Wed, Sep 21, 2016 at 10:24 PM, Jeff King <peff@peff.net> wrote:
> When cloning with "--recursive", we'd generally expect
> submodules to show progress reports if the main clone did,
> too.
>
> In older versions of git, this mostly worked out of the
> box. Since we show progress by default when stderr is a tty,
> and since the child clones inherit the parent stderr, then
> both processes would come to the same decision by default.
> If the parent clone was asked for "--quiet", we passed down
> "--quiet" to the child. However, if stderr was not a tty and
> the user specified "--progress", we did not propagate this
> to the child.
>
> That's a minor bug, but things got much worse when we
> switched recently to submodule--helper's update_clone
> command. With that change, the stderr of the child clones
> are always connected to a pipe, and we never output
> progress at all.
Right, that is an issue.
>
> Signed-off-by: Jeff King <peff@peff.net>
Acked and thanked by Stefan ;)
>
> I imagine there are other code paths that want similar treatment, but I
> didn't look into them. I'd assume "fetch" is one. I'm not sure if we do
> parallel checkouts, but that's another potential.
Looking for run_processes_parallel in the code,
it seems to only be used in fetch_populated_submodules
and the submodule helper.
>
> update_head(our_head_points_at, remote_head, reflog_msg.buf);
>
> + /*
> + * We want to show progress for recursive submodule clones iff
> + * we did so for the main clone. But only the transport knows
> + * the final decision for this flag, so we need to rescue the value
> + * before we free the transport.
> + */
> + submodule_progress = transport->progress;
> +
Good point! I was aware of this bug (but I did not consider it to be impactful
or as you put it "much worse"), but I anticipated we would need some refactoring
of the transport code, e.g. have the decision via isatty(2) as a
separate outside
function that we consult before we even setup the transport and then
pass it down
to the submodules as well. This seems to solve this bug elegantly.
> transport_unlock_pack(transport);
> transport_disconnect(transport);
>
> @@ -1108,7 +1120,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> }
>
> junk_mode = JUNK_LEAVE_REPO;
> - err = checkout();
> + err = checkout(submodule_progress);
>
> strbuf_release(&reflog_msg);
> strbuf_release(&branch_top);
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 7b8ddfe..d2f9d7d 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -443,7 +443,8 @@ static int module_name(int argc, const char **argv, const char *prefix)
> }
>
> static int clone_submodule(const char *path, const char *gitdir, const char *url,
> - const char *depth, struct string_list *reference, int quiet)
> + const char *depth, struct string_list *reference,
> + int quiet, int progress)
I am not sure if having both quiet and progress is maintainable well,
but it get's the job done here, specifically if we consider this patch a bug
fix that we'd want to merge down to maint.
^ permalink raw reply
* .gitignore does not ignore Makefile
From: Timur Tabi @ 2016-09-22 14:19 UTC (permalink / raw)
To: git
I have the following .gitignore file in patch arm/arm64/boot/dts:
*.dtb
qcom
qcom.orig
When I do a git status, I see this:
modified: .gitignore
modified: qcom/Makefile
All of the other files in arm/arm64/boot/dts/qcom are being ignored,
as request. However, the file "Makefile" is not being ignored. Why?
What's so special about "Makefile" that git refuses to ignore it?
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH 3/3] docs/cvs-migration: mention cvsimport caveats
From: Eric S. Raymond @ 2016-09-22 13:15 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20160922072628.rs47hfiowtobw46s@sigill.intra.peff.net>
Jeff King <peff@peff.net>:
> Back when this guide was written, cvsimport was the only
> game in town. These days it is probably not the best option.
It is absolutely not. As I have tried to point out here before, it
is *severely* broken in its processing of branchy CVS repositories.
Nobody wanted to hear that, but it's still true. Recommending it
is irresponsible.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: v2.10.0: ls-tree exit status is always 0, this differs from ls(1)
From: Steffen Nurpmeso @ 2016-09-22 12:57 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <68354d78-fa7a-ee99-2e6e-7ffdcf1a568e@drmicha.warpmail.net>
Hello,
Michael J Gruber <git@drmicha.warpmail.net> wrote:
|Steffen Nurpmeso venit, vidit, dixit 22.09.2016 00:46:
|> Junio C Hamano <gitster@pobox.com> wrote:
|>|Steffen Nurpmeso <steffen@sdaoden.eu> writes:
...
|>|I think this issue does not need a separate bullet point. The
|>|existing text says:
|> ..
|>|and what caused your surprise is already covered by the first bullet
|>|point, if the reader knows what "patterns to match" means in Git's
...
|>|How about rewriting the first bullet point like so instead:
...
|>| of the arguments does not matter, and a '<path>' argument that
|>| does not match any path is not an error (i.e. if there is no
|>| path that matches any pattern, nothing is shown in the output).
|>
|> Not an error would have been an enlightenment to me.
...
|>
|> But now i'm even getting nervous to read about patterns.
...
|> We have patterns for tags/remotes/branches, author/committer/grep
|> patterns, (most of those, maybe all today, with fixed string,
|> extended or basic regex), the git-grep patterns ("leading paths
|> match and glob(7) patterns are supported"). Is that all?
|> I would assume glob-style for ls-tree:
...
|Maybe "git ls-files" is the command that you are looking for, really.
|
|That and others have glob pathspec enabled by default, see "git help git".
Please rollback all of that, i have only reported something that
seemed odd to me. What i really need is instead
if `git cat-file -e ${relbr}:NEWS 2>/dev/null`; then
and that is what i will end up with.
_But_, now that i am here again, "git help cat-file" says
-e
Suppress all output; instead exit with zero status if <object>
exists and is a valid object.
and
OUTPUT
...
If -e is specified, no output.
But this is not what happens if "output" includes stderr:
?0[steffen@wales ]$ git cat-file -e HEAD:NEWS
?0[steffen@wales ]$ git cat-file -e HEAD:NEWSS
fatal: Not a valid object name HEAD:NEWSS
?128[steffen@wales ]$
I would also not expect $?=128 as an counterpart to EXIT_SUCCESS=0
when performing a qualified "test" action, but EXIT_FAILURE=1 is
just an as-bad non-0 exit status code, anyway. To me
EX_NOINPUT=66 obtrudes itself as the right status, but my own
projects don't adhere to this from a-z or not at all, so what i am
talking about? I mean, some things take time and are eventually
and temporarily a bit odd, so what? That is just how it is. Even
Sparta declined some day, and then it crushed, iirc.
Thanks for git, just yesterday evening i did rebasing and cherry
picking and commit amending and garbage collection and it saved me
days of work, or, to be more exact, i never have been able to work
the way i would work before. Really.
Ciao.
--steffen
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox