Re: [PATCH v2 00/19] Index-v5
Ramsay Jones writes:
> Thomas Gummerer wrote:
>> Hi,
>>
>> previous rounds (without api) are at $gmane/202752, $gmane/202923,
>> $gmane/203088 and $gmane/203517, the previous round with api was at
>> $gmane/229732. Thanks to Junio, Duy and Eric for their comments on
>> the previous round.
>
> If I remember correctly, the original version of this series had the
> same problem as Michael's "Fix some reference-related races" series
> (now in master). In particular, you had introduced an 'index_changed()'
> function which does essentially the same job as 'stat_validity_check()'
> in the new reference handling API. I seem to remember advising you
> not to compare st_uid, st_gid and st_ino on __CYGWIN__.
Yes, you provided a patch that simply wrapped those checks in a #if
!defined (__CYGWIN__), which is included in the new series too.
> I haven't had time to look at this version of your series yet, but it
> may be worth taking a look at stat_validity_check(). (although that is
> causing failures on cygwin at the moment! ;-)
I took a quick look, that function makes sense I think. I'll use it in
the re-roll. It makes probably sense to wrap the uid, gid and ino
fields as in the index_changed function.
> Also, I can't recall if I mentioned it to you at the time, but your
> index reading code was (unnecessarily) calling munmap() twice on the
> same buffer (without an intervening mmap()). This causes problems for
> systems that have the NO_MMAP build variable set. In particular, the
> compat/mmap.c code will attempt to free() the allocated memory block
> twice, with unpredictable results.
>
> I wrote a patch to address this at the time (Hmm, seems to be built
> on v1.8.1), but didn't submit it since your patch didn't progress. :-D
> I have included the patch below.
I can't recall this either. From a quick check I don't call munmap() on
a already unmapped mmap, so I think this is fine as it is and your patch
is independent from it. Not sure if it makes sense as safeguard for
future changes.
> -- >8 --
> From: Ramsay Jones
> Date: Sun, 9 Sep 2012 20:50:32 +0100
> Subject: [PATCH] mmap.c: Keep log of mmap() blocks to avoid double-delete bug
>
> When compiling with the NO_MMAP build variable set, the built-in
> 'git_mmap()' and 'git_munmap()' compatibility routines use simple
> memory allocation and file I/O to emulate the required behaviour.
> The current implementation is vulnerable to the "double-delete" bug
> (where the pointer returned by malloc() is passed to free() two or
> more times), should the mapped memory block address be passed to
> munmap() multiple times.
>
> In order to guard the implementation from such a calling sequence,
> we keep a list of mmap-block descriptors, which we then consult to
> determine the validity of the input pointer to munmap(). This then
> allows 'git_munmap()' to return -1 on error, as required, with
> errno set to EINVAL.
>
> Using a list in the log of mmap-ed blocks, along with the resulting
> linear search, means that the performance of the code is directly
> proportional to the number of concurrently active memory mapped
> file regions. The number of such regions is not expected to be
> excessive.
>
> Signed-off-by: Ramsay Jones
> ---
> compat/mmap.c | 57 -
> 1 file changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/compat/mmap.c b/compat/mmap.c
> index c9d46d1..400e034 100644
> --- a/compat/mmap.c
> +++ b/compat/mmap.c
> @@ -1,14 +1,61 @@
> #include "../git-compat-util.h"
>
> +struct mmbd { /* memory mapped block descriptor */
> + struct mmbd *next; /* next in list */
> + void *start; /* pointer to memory mapped block */
> + size_t length; /* length of memory mapped block */
> +};
> +
> +static struct mmbd *head; /* head of mmb descriptor list */
> +
> +
> +static void add_desc(struct mmbd *desc, void *start, size_t length)
> +{
> + desc->start = start;
> + desc->length = length;
> + desc->next = head;
> + head = desc;
> +}
> +
> +static void free_desc(struct mmbd *desc)
> +{
> + if (head == desc)
> + head = head->next;
> + else {
> + struct mmbd *d = head;
> + for (; d; d = d->next) {
> + if (d->next == desc) {
> + d->next = desc->next;
> + break;
> + }
> + }
> + }
> + free(desc);
> +}
> +
> +static struct mmbd *find_desc(void *start)
> +{
> + struct mmbd *d = head;
> + for (; d; d = d->next) {
> + if (d->start == start)
> + return d;
> + }
> + return NULL;
> +}
> +
> void *git_mmap(void *start, size_t length, int prot, int flags, int fd,
> off_t offset)
> {
> size_t n = 0;
> + struct mmbd *desc = NULL;
>
> if (start != NULL || !(flags & MAP_PRIVATE))
> die("Invalid usage of mmap when built with NO_MMAP");
>
> s
Re: [PATCH v2 09/19] ls-files.c: use index api
Duy Nguyen writes:
> On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer
> wrote:
>> + if (!with_tree) {
>> + memset(opts, 0, sizeof(*opts));
>> + opts->pathspec = &pathspec_struct;
>> + opts->read_staged = 1;
>> + if (show_resolve_undo)
>> + opts->read_resolve_undo = 1;
>> + read_cache_filtered(opts);
>
> So you load partial index here.
>
>> + } else {
>> + read_cache();
>> + }
>> + /* be nice with submodule paths ending in a slash */
>> + if (pathspec)
>> + strip_trailing_slash_from_submodules();
>
> Then strip_trailing_slash_from_submodules will attempt to convert
> pathspec "foo/" to "foo" if "foo" exists in the index and is a
> gitlink. But becaues "foo/" is used to load the partial index, "foo"
> is not loaded (is it?) and this could become incorrect no-op. I
> suggest you go through the pathspec once checking for ones ending with
> '/'. If so strip_trailing_... may potentially update pathspec, just
> load full index. If no pathspec ends with '/', strip_trail... is no-op
> and we can do partial loading safely.
It was loaded, because the adjusted_pathspec algorithm stripped the
trailing slash and then everything until the next slash. That's
overkill except when the trailing slash had to be stripped.
I'll make the adjusted_pathspec algorithm more restrictive, so the last
trailing slash is no longer stripped. If a pathspec contains a trailing
slash I'll load the whole index, as you suggested.
--
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 10/19] documentation: add documentation of the index-v5 file format
Duy Nguyen writes:
> On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer
> wrote:
>> +== Directory offsets (diroffsets)
>> +
>> + diroffset (32-bits): offset to the directory relative to the beginning
>> +of the index file. There are ndir + 1 offsets in the diroffset table,
>> +the last is pointing to the end of the last direntry. With this last
>> +entry, we are able to replace the strlen of when reading the directory
>
> strlen of what?
Of the dirname. Will fix in the re-roll, thanks for noticing.
>> +name, by calculating it from diroffset[n+1]-diroffset[n]-61. 61 is the
>> +size of the directory data, which follows each each directory + the
>> +crc sum + the NUL byte.
>> +
>> + This part is needed for making the directory entries bisectable and
>> +thus allowing a binary search.
>
> Just thinking out loud. Maybe this section and fileoffsets should be
> made optional extensions. So far I see no use for them. It's nice for
> a program to occasionally look at a single entry, but such programs do
> not exist (yet). For inotify monitor that may want to update a single
> file's stat, it could regenerate the index with {dir,file}offsets
> extensions the first time it attempts to update the index, then it
> could do bsearch.
There is a use already, namely saving the strlen for the filename.
Previously we had the length of the filename in the lower bits of the
flags, but decided to remove it to avoid having extended flags. That in
addition to the use case in the future warrants keeping them in the
index, I think.
--
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 12/19] read-cache: read index-v5
Duy Nguyen writes:
[..]
>> +static int read_entries(struct index_state *istate, struct directory_entry
>> **de,
>> + unsigned int *entry_offset, void **mmap,
>> + unsigned long mmap_size, unsigned int *nr,
>> + unsigned int *foffsetblock)
>> +{
>> + struct cache_entry *head = NULL, *tail = NULL;
>> + struct conflict_entry *conflict_queue;
>> + struct cache_entry *ce;
>> + int i;
>> +
>> + conflict_queue = NULL;
>> + if (read_conflicts(&conflict_queue, *de, mmap, mmap_size) < 0)
>> + return -1;
>> + for (i = 0; i < (*de)->de_nfiles; i++) {
>> + if (read_entry(&ce,
>> + *de,
>> + entry_offset,
>> + mmap,
>> + mmap_size,
>> + foffsetblock) < 0)
>> + return -1;
>> + ce_queue_push(&head, &tail, ce);
>> + *foffsetblock += 4;
>> +
>> + /*
>> +* Add the conflicted entries at the end of the index file
>> +* to the in memory format
>> +*/
>> + if (conflict_queue &&
>> + (conflict_queue->entries->flags & CONFLICT_CONFLICTED)
>> != 0 &&
>> + !cache_name_compare(conflict_queue->name,
>> conflict_queue->namelen,
>> + ce->name, ce_namelen(ce))) {
>> + struct conflict_part *cp;
>> + cp = conflict_queue->entries;
>> + cp = cp->next;
>> + while (cp) {
>> + ce = convert_conflict_part(cp,
>> + conflict_queue->name,
>> + conflict_queue->namelen);
>> + ce_queue_push(&head, &tail, ce);
>> + conflict_part_head_remove(&cp);
>> + }
>> + conflict_entry_head_remove(&conflict_queue);
>> + }
>
> I start to wonder if separating staged entries is a good idea. It
> seems to make the code more complicated. The good point about conflict
> section at the end of the file is you can just truncate() it out.
> Another way is putting staged entries in fileentries, sorted
> alphabetically then by stage number, and a flag indicating if the
> entry is valid. When you remove resolve an entry, just set the flag to
> invalid (partial write), so that read code will skip it.
>
> I think this approach is reasonably cheap (unless there are a lot of
> conflicts) and it simplifies this piece of code. truncate() may be
> overrated anyway. In my experience, I "git add " as soon as I
> resolve (so that "git diff" shrinks). One entry at a time, one
> index write at a time. I don't think I ever resolve everything then
> "git add -u .", which is where truncate() shines because staged
> entries are removed all at once. We should optimize for one file
> resolution at a time, imo.
Thanks for your comments. I'll address the other ones once we decided
to do with the conflicts.
It does make the code quite a bit more complicated, but also has one
advantage that you overlooked. We wouldn't truncate() when resolving
the conflicts. The resolve undo data is stored with the conflicts and
therefore we could just flip a bit and set the stage of the cache-entry
in the main index to 0 (always once we got partial writing). This can
be fast both in case we resolve one entry at a time and when we resolve
a lot of entries. The advantage is even bigger when we resolve one
entry at a time, when we otherwise would have to re-write the index for
each conflict resolution.
[..]
--
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/19] Index-v5
Duy Nguyen writes: > On Mon, Jul 15, 2013 at 4:30 PM, Thomas Gummerer wrote: >> Duy Nguyen writes: >> >>> On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer >>> wrote: t/perf/p0003-index.sh| 59 + t/t2104-update-index-skip-worktree.sh|1 + >>> >>> For such a big code addition, the test part seems modest. Speaking >>> from my experience, I rarely run perf tests and "make test" does not >>> activate v5 code at all. A few more tests would be nice. The good news >>> is I changed default index version to 5 and ran "make test", all >>> passed. >> >> I was using the test suite with index version 5 as default index version >> for testing of the new index file format. I think that's the best way >> to test the index, as it covers all aspects. > > We need other people to run the test suite with v5 to catch bugs that > only appear on other platforms. Perhaps an env variable to allow the > test suite to override the binary's default index version and a new > make target "test-v5" maybe. Ah ok, I understand. I think it's best to add a GIT_INDEX_VERSION=x config option to config.mak, where x is the index version that should be tested. This is also the way other test options are set (e.g. NO_SVN_TESTS). In addition this allows testing other versions of the index file too. >> Maybe we should add a test >> that covers the basic functionality, just to make sure nothing obvious >> is broken when running the test suite with index-v2? Something like >> this maybe: > > Yep. v5 specfic test cases can cover some corner cases that the rest > of the test suite does not care. Haven't read your t1600 though. Ok. I can't think of any corner cases right now that would be in v5, but not in other versions, but if I catch some I'll add tests. -- 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: [PATCHv3 1/3] l10n: de.po: switch from pure German to German+English (part 1)
Ralf Thielow writes:
> This switches the translation from pure German to German+English.
>
I tried to compare this to the old version, but your patch is damaged at
line 160 counting from this:
> ---
> po/de.po | 568
> +++
> 1 file changed, 284 insertions(+), 284 deletions(-)
[...]
> #: branch.c:137
> #, c-format
> msgid "Not tracking: ambiguous information for ref %s"
> -msgstr "Konfiguration zum Folgen von Zweig nicht
> eingerichtet. Referenz %s ist mehrdeutig."
> +msgstr "Konfiguration zum Folgen von Branch nicht
> eingerichtet. Referenz %s ist mehrdeutig."
These two lines apparently got wrapped by your MUA. There are more
instances of the same damage. Can you send a fixed patch?
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Missing capabilities in Documentation/technical/protocol-capbilities.txt
On Mon, Jul 15, 2013 at 07:25:19PM +0700, Duy Nguyen wrote: > > I noticed that "quiet" and "agent" capabilities were missing in > protocol-capabilitities.txt. I have a rough idea what they do, but I > think it's best to be documented by the authors. Maybe you have some > time to make a patch? Hi Duy, I am sorry to disappoint, but if I had time to work on Git, I'd rather be writing code. I have some great ideas if you are interested. :-P Besides, I barely even remember that it was me who implemented the "quiet" capability. In order to write documentation for it, I would have to research the implementation as much as anyone. Cheers, Clemens -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] First class shallow clone
This is probably the first attempt to treat shallow clones just like
ordinary ones. Which means you can push or fetch/clone between any two
repos, regardless of their shallow status. There are two purposes
behind this:
- to make local/shallow clone <-> (complete) upstream repo workflow
smoother, the note about shallow clone limitation in git-clone.txt
can be removed
- to make it possible for upstream to provide a lightweight repo that
others can use. For example, big repos with lots of activities can
be split into new base repo that only contains the work of maximum one
year and a complete repo mostly for archive.
This is a naive approach. I might overlook something again, which is
why I publish it early to get more eyes on it.
The idea is simple: in shallow case, we provide the pack _and_
.git/shallow file to the other end. The other end will setup extra
grafting to make sure the updated repo is "complete". More in
individual patch messages.
There might be issues with generating optimum pack for transfer when
both ends are shallow.. There's also an interesting issue, whether we
can take advantage of commit bitmaps in shallow clones if they are
more widely used.. We also might need a config key to protect a repo
from becoming shallow by a fetch or push, if the repo is to be a
backup one..
Nguyễn Thái Ngọc Duy (7):
transport.h: remove send_pack prototype, already defined in send-pack.h
{receive,upload}-pack: advertise shallow graft information
connect.c: teach get_remote_heads to parse "shallow" lines
Move setup_alternate_shallow and write_shallow_commits to shallow.c
fetch-pack: support fetching from a shallow repository
{send,receive}-pack: support pushing from a shallow clone
send-pack: support pushing to a shallow clone
Documentation/technical/pack-protocol.txt | 7 +-
builtin/fetch-pack.c | 6 +-
builtin/receive-pack.c| 54
builtin/send-pack.c | 7 +-
cache.h | 1 +
commit.h | 8 +++
connect.c | 12 +++-
fetch-pack.c | 78 +++---
fetch-pack.h | 1 +
remote-curl.c | 2 +-
send-pack.c | 41 ++--
send-pack.h | 4 +-
shallow.c | 103 ++
t/t5536-fetch-shallow.sh (new +x) | 75 ++
t/t5537-push-shallow.sh (new +x) | 85
transport.c | 14 ++--
transport.h | 6 --
upload-pack.c | 3 +-
18 files changed, 414 insertions(+), 93 deletions(-)
create mode 100755 t/t5536-fetch-shallow.sh
create mode 100755 t/t5537-push-shallow.sh
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/7] transport.h: remove send_pack prototype, already defined in send-pack.h
Signed-off-by: Nguyễn Thái Ngọc Duy --- transport.h | 6 -- 1 file changed, 6 deletions(-) diff --git a/transport.h b/transport.h index ea70ea7..3178dc9 100644 --- a/transport.h +++ b/transport.h @@ -182,10 +182,4 @@ void transport_print_push_status(const char *dest, struct ref *refs, typedef void alternate_ref_fn(const struct ref *, void *); extern void for_each_alternate_ref(alternate_ref_fn, void *); - -struct send_pack_args; -extern int send_pack(struct send_pack_args *args, -int fd[], struct child_process *conn, -struct ref *remote_refs, -struct extra_have_objects *extra_have); #endif -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/7] {send,receive}-pack: support pushing from a shallow clone
Pushing from a shallow clone using today's send-pack and receive-pack
may work, if the transferred pack does not ends up at any graft
points. If it does, recent receive-pack that does connectivity check
will reject the push. If receive-pack is old, the upstream repo
becomes corrupt.
The pack protocol is updated and send-pack now sends all shallow
grafts before it sends the commands, if the repo is shallow. This
protocol extension will break current receive-pack, which is intended,
mostly to stop corrupting the upstream repo.
The receiver end, the newreceive-pack, does something similar to
fetch-pack: it creates a temporary shallow file with grafts from
send-pack, then receives the pack, and finally writes the refined
shallow file down.
shadow file is not cleaned up after deleting (or force updating) a ref
if that ref is the only way to reach the graft points. The reason is
once we delete graft points, we can't recover. That may make reflog
entries on server useless. Leave that for the administrators to decide
when to clean up shadow file (maybe at repack/gc time).
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/technical/pack-protocol.txt | 4 +-
builtin/receive-pack.c| 49 ++
send-pack.c | 3 ++
t/t5537-push-shallow.sh (new +x) | 67 +++
4 files changed, 114 insertions(+), 9 deletions(-)
create mode 100755 t/t5537-push-shallow.sh
diff --git a/Documentation/technical/pack-protocol.txt
b/Documentation/technical/pack-protocol.txt
index eb8edd1..c73b62f 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -464,7 +464,9 @@ contain all the objects that the server will need to
complete the new
references.
- update-request= command-list [pack-file]
+ update-request= *shallow command-list [pack-file]
+
+ shallow = PKT-LINE("shallow" SP obj-id)
command-list = PKT-LINE(command NUL capability-list LF)
*PKT-LINE(command LF)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 2d8e19b..0537e26 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -41,6 +41,10 @@ static int auto_gc = 1;
static const char *head_name;
static void *head_name_to_free;
static int sent_capabilities;
+static int shallow_changed;
+static const char* alternate_shallow_file;
+static struct lock_file shallow_lock;
+static struct extra_have_objects shallow;
static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -751,6 +755,13 @@ static void execute_commands(struct command *commands,
const char *unpacker_erro
}
}
+static void add_extra_have(struct extra_have_objects *extra, unsigned char
*sha1)
+{
+ ALLOC_GROW(extra->array, extra->nr + 1, extra->alloc);
+ hashcpy(&(extra->array[extra->nr][0]), sha1);
+ extra->nr++;
+}
+
static struct command *read_head_info(void)
{
struct command *commands = NULL;
@@ -765,6 +776,17 @@ static struct command *read_head_info(void)
line = packet_read_line(0, &len);
if (!line)
break;
+
+ if (len == 48 && !prefixcmp(line, "shallow ")) {
+ if (get_sha1_hex(line + 8, old_sha1))
+ die("protocol error: expected shallow sha, got
'%s'", line + 8);
+ if (!has_sha1_file(old_sha1)) {
+ add_extra_have(&shallow, old_sha1);
+ shallow_changed = 1;
+ }
+ continue;
+ }
+
if (len < 83 ||
line[40] != ' ' ||
line[81] != ' ' ||
@@ -827,6 +849,12 @@ static const char *unpack(int err_fd)
? transfer_fsck_objects
: 0);
+ if (shallow_changed)
+ setup_alternate_shallow(&shallow_lock,
+ &alternate_shallow_file,
+ &shallow, 0);
+
+
hdr_err = parse_pack_header(&hdr);
if (hdr_err) {
if (err_fd > 0)
@@ -854,9 +882,8 @@ static const char *unpack(int err_fd)
child.err = err_fd;
child.git_cmd = 1;
code = run_command(&child);
- if (!code)
- return NULL;
- return "unpack-objects abnormal exit";
+ if (code)
+ return "unpack-objects abnormal exit";
} else {
const char *keeper[7];
int s, status, i = 0;
@@ -887,12 +914,18 @@ static const char *unpack(int err_fd)
pack_lockfile = index_pack_lockfile(ip.out);
close(ip.out);
status = finish_command(&ip);
- if (!status) {
-
[PATCH 4/7] Move setup_alternate_shallow and write_shallow_commits to shallow.c
Signed-off-by: Nguyễn Thái Ngọc Duy
---
commit.h | 3 +++
fetch-pack.c | 53 +
shallow.c| 53 +
3 files changed, 57 insertions(+), 52 deletions(-)
diff --git a/commit.h b/commit.h
index e0688c3..678fa20 100644
--- a/commit.h
+++ b/commit.h
@@ -188,6 +188,9 @@ extern struct commit_list *get_shallow_commits(struct
object_array *heads,
extern void check_shallow_file_for_update(void);
extern void set_alternate_shallow_file(const char *path);
extern void advertise_shallow_grafts(int);
+extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol);
+extern void setup_alternate_shallow(struct lock_file *shallow_lock,
+ const char **alternate_shallow_file);
int is_descendant_of(struct commit *, struct commit_list *);
int in_merge_bases(struct commit *, struct commit *);
diff --git a/fetch-pack.c b/fetch-pack.c
index abe5ffb..dc71a2b 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -185,36 +185,6 @@ static void consume_shallow_list(struct fetch_pack_args
*args, int fd)
}
}
-struct write_shallow_data {
- struct strbuf *out;
- int use_pack_protocol;
- int count;
-};
-
-static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
-{
- struct write_shallow_data *data = cb_data;
- const char *hex = sha1_to_hex(graft->sha1);
- data->count++;
- if (data->use_pack_protocol)
- packet_buf_write(data->out, "shallow %s", hex);
- else {
- strbuf_addstr(data->out, hex);
- strbuf_addch(data->out, '\n');
- }
- return 0;
-}
-
-static int write_shallow_commits(struct strbuf *out, int use_pack_protocol)
-{
- struct write_shallow_data data;
- data.out = out;
- data.use_pack_protocol = use_pack_protocol;
- data.count = 0;
- for_each_commit_graft(write_one_shallow, &data);
- return data.count;
-}
-
static enum ack_type get_ack(int fd, unsigned char *result_sha1)
{
int len;
@@ -795,27 +765,6 @@ static int cmp_ref_by_name(const void *a_, const void *b_)
return strcmp(a->name, b->name);
}
-static void setup_alternate_shallow(void)
-{
- struct strbuf sb = STRBUF_INIT;
- int fd;
-
- check_shallow_file_for_update();
- fd = hold_lock_file_for_update(&shallow_lock, git_path("shallow"),
- LOCK_DIE_ON_ERROR);
- if (write_shallow_commits(&sb, 0)) {
- if (write_in_full(fd, sb.buf, sb.len) != sb.len)
- die_errno("failed to write to %s",
shallow_lock.filename);
- alternate_shallow_file = shallow_lock.filename;
- } else
- /*
-* is_repository_shallow() sees empty string as "no
-* shallow file".
-*/
- alternate_shallow_file = "";
- strbuf_release(&sb);
-}
-
static struct ref *do_fetch_pack(struct fetch_pack_args *args,
int fd[2],
const struct ref *orig_ref,
@@ -896,7 +845,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args
*args,
if (args->stateless_rpc)
packet_flush(fd[1]);
if (args->depth > 0)
- setup_alternate_shallow();
+ setup_alternate_shallow(&shallow_lock, &alternate_shallow_file);
if (get_pack(args, fd, pack_lockfile))
die("git fetch-pack: fetch failed.");
diff --git a/shallow.c b/shallow.c
index ccdfefc..ee9edd4 100644
--- a/shallow.c
+++ b/shallow.c
@@ -162,3 +162,56 @@ void advertise_shallow_grafts(int fd)
return;
for_each_commit_graft(advertise_shallow_grafts_cb, &fd);
}
+
+struct write_shallow_data {
+ struct strbuf *out;
+ int use_pack_protocol;
+ int count;
+};
+
+static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
+{
+ struct write_shallow_data *data = cb_data;
+ const char *hex = sha1_to_hex(graft->sha1);
+ data->count++;
+ if (data->use_pack_protocol)
+ packet_buf_write(data->out, "shallow %s", hex);
+ else {
+ strbuf_addstr(data->out, hex);
+ strbuf_addch(data->out, '\n');
+ }
+ return 0;
+}
+
+int write_shallow_commits(struct strbuf *out, int use_pack_protocol)
+{
+ struct write_shallow_data data;
+ data.out = out;
+ data.use_pack_protocol = use_pack_protocol;
+ data.count = 0;
+ for_each_commit_graft(write_one_shallow, &data);
+ return data.count;
+}
+
+void setup_alternate_shallow(struct lock_file *shallow_lock,
+const char **alternate_shallow_file)
+{
+ struct strbuf sb = STRBUF_INIT;
+ int fd;
+
+ check_shallow_file_for_update();
+ fd = hold_lock_file_for_update(shallow_lock, g
[PATCH 2/7] {receive,upload}-pack: advertise shallow graft information
If either receive-pack or upload-pack is called on a shallow
repository, shallow graft points will be sent after the ref
advertisement (but before the packet flush).
This breaks the protocol for all clients trying to push to a shallow
repo, or fetch from one. Which is basically the same end result as
today's "is_repository_shallow() && die()" in receive-pack and
upload-pack. New clients will be made aware of shallow upstream and
can make use of this information.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/technical/pack-protocol.txt | 3 +++
builtin/receive-pack.c| 5 ++---
commit.h | 1 +
shallow.c | 16
upload-pack.c | 3 +--
5 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/Documentation/technical/pack-protocol.txt
b/Documentation/technical/pack-protocol.txt
index b898e97..eb8edd1 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -161,6 +161,7 @@ MUST peel the ref if it's an annotated tag.
advertised-refs = (no-refs / list-of-refs)
+ *shallow
flush-pkt
no-refs = PKT-LINE(zero-id SP "capabilities^{}"
@@ -174,6 +175,8 @@ MUST peel the ref if it's an annotated tag.
other-tip= obj-id SP refname LF
other-peeled = obj-id SP refname "^{}" LF
+ shallow = PKT-LINE("shallow" SP obj-id)
+
capability-list = capability *(SP capability)
capability = 1*(LC_ALPHA / DIGIT / "-" / "_")
LC_ALPHA = %x61-7A
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e3eb5fc..2d8e19b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -176,6 +176,8 @@ static void write_head_info(void)
if (!sent_capabilities)
show_ref("capabilities^{}", null_sha1);
+ advertise_shallow_grafts(1);
+
/* EOF */
packet_flush(1);
}
@@ -990,9 +992,6 @@ int cmd_receive_pack(int argc, const char **argv, const
char *prefix)
if (!enter_repo(dir, 0))
die("'%s' does not appear to be a git repository", dir);
- if (is_repository_shallow())
- die("attempt to push into a shallow repository");
-
git_config(receive_pack_config, NULL);
if (0 <= transfer_unpack_limit)
diff --git a/commit.h b/commit.h
index 4d452dc..e0688c3 100644
--- a/commit.h
+++ b/commit.h
@@ -187,6 +187,7 @@ extern struct commit_list *get_shallow_commits(struct
object_array *heads,
int depth, int shallow_flag, int not_shallow_flag);
extern void check_shallow_file_for_update(void);
extern void set_alternate_shallow_file(const char *path);
+extern void advertise_shallow_grafts(int);
int is_descendant_of(struct commit *, struct commit_list *);
int in_merge_bases(struct commit *, struct commit *);
diff --git a/shallow.c b/shallow.c
index cbe2526..ccdfefc 100644
--- a/shallow.c
+++ b/shallow.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "commit.h"
#include "tag.h"
+#include "pkt-line.h"
static int is_shallow = -1;
static struct stat shallow_stat;
@@ -146,3 +147,18 @@ void check_shallow_file_for_update(void)
)
die("shallow file was changed during fetch");
}
+
+static int advertise_shallow_grafts_cb(const struct commit_graft *graft, void
*cb)
+{
+ int fd = *(int*)cb;
+ if (graft->nr_parent == -1)
+ packet_write(fd, "shallow %s\n", sha1_to_hex(graft->sha1));
+ return 0;
+}
+
+void advertise_shallow_grafts(int fd)
+{
+ if (!is_repository_shallow())
+ return;
+ for_each_commit_graft(advertise_shallow_grafts_cb, &fd);
+}
diff --git a/upload-pack.c b/upload-pack.c
index 127e59a..6cefe43 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -766,6 +766,7 @@ static void upload_pack(void)
reset_timeout();
head_ref_namespaced(send_ref, NULL);
for_each_namespaced_ref(send_ref, NULL);
+ advertise_shallow_grafts(1);
packet_flush(1);
} else {
head_ref_namespaced(mark_our_ref, NULL);
@@ -837,8 +838,6 @@ int main(int argc, char **argv)
if (!enter_repo(dir, strict))
die("'%s' does not appear to be a git repository", dir);
- if (is_repository_shallow())
- die("attempt to fetch/clone from a shallow repository");
git_config(upload_pack_config, NULL);
upload_pack();
return 0;
--
1.8.2.83.gc99314b
--
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 3/7] connect.c: teach get_remote_heads to parse "shallow" lines
No callers pass a non-empty pointer as shallow_points at this
stage. As a result, all clients still refuse to talk to shallow
repository on the other end.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/fetch-pack.c | 2 +-
builtin/send-pack.c | 2 +-
cache.h | 1 +
connect.c| 12 +++-
remote-curl.c| 2 +-
transport.c | 7 ---
6 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index aba4465..080e599 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -144,7 +144,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char
*prefix)
args.verbose ? CONNECT_VERBOSE : 0);
}
- get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL);
+ get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, NULL);
ref = fetch_pack(&args, fd, conn, ref, dest,
sought, nr_sought, pack_lockfile_ptr);
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 152c4ea..f0ad0ce 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -207,7 +207,7 @@ int cmd_send_pack(int argc, const char **argv, const char
*prefix)
memset(&extra_have, 0, sizeof(extra_have));
- get_remote_heads(fd[0], NULL, 0, &remote_refs, REF_NORMAL, &extra_have);
+ get_remote_heads(fd[0], NULL, 0, &remote_refs, REF_NORMAL, &extra_have,
NULL);
transport_verify_remote_names(nr_refspecs, refspecs);
diff --git a/cache.h b/cache.h
index dd0fb33..7665e03 100644
--- a/cache.h
+++ b/cache.h
@@ -1091,6 +1091,7 @@ struct extra_have_objects {
};
extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
struct ref **list, unsigned int flags,
+struct extra_have_objects *,
struct extra_have_objects *);
extern int server_supports(const char *feature);
extern int parse_feature_request(const char *features, const char *feature);
diff --git a/connect.c b/connect.c
index a0783d4..1cba424 100644
--- a/connect.c
+++ b/connect.c
@@ -64,7 +64,8 @@ static void die_initial_contact(int got_at_least_one_head)
*/
struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
struct ref **list, unsigned int flags,
- struct extra_have_objects *extra_have)
+ struct extra_have_objects *extra_have,
+ struct extra_have_objects *shallow_points)
{
int got_at_least_one_head = 0;
@@ -89,6 +90,15 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t
src_len,
if (len > 4 && !prefixcmp(buffer, "ERR "))
die("remote error: %s", buffer + 4);
+ if (len == 48 && !prefixcmp(buffer, "shallow ")) {
+ if (get_sha1_hex(buffer + 8, old_sha1))
+ die("protocol error: expected shallow sha, got
'%s'", buffer + 8);
+ if (!shallow_points)
+ die("repository on the other end cannot be
shallow");
+ add_extra_have(shallow_points, old_sha1);
+ continue;
+ }
+
if (len < 42 || get_sha1_hex(buffer, old_sha1) || buffer[40] !=
' ')
die("protocol error: expected sha/ref, got '%s'",
buffer);
name = buffer + 41;
diff --git a/remote-curl.c b/remote-curl.c
index 5b3ce9e..c329bd3 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -86,7 +86,7 @@ static struct ref *parse_git_refs(struct discovery *heads,
int for_push)
{
struct ref *list = NULL;
get_remote_heads(-1, heads->buf, heads->len, &list,
-for_push ? REF_NORMAL : 0, NULL);
+for_push ? REF_NORMAL : 0, NULL, NULL);
return list;
}
diff --git a/transport.c b/transport.c
index e15db98..10a8cb8 100644
--- a/transport.c
+++ b/transport.c
@@ -509,7 +509,7 @@ static struct ref *get_refs_via_connect(struct transport
*transport, int for_pus
connect_setup(transport, for_push, 0);
get_remote_heads(data->fd[0], NULL, 0, &refs,
-for_push ? REF_NORMAL : 0, &data->extra_have);
+for_push ? REF_NORMAL : 0, &data->extra_have, NULL);
data->got_remote_heads = 1;
return refs;
@@ -539,7 +539,8 @@ static int fetch_refs_via_pack(struct transport *transport,
if (!data->got_remote_heads) {
connect_setup(transport, 0, 0);
- get_remote_heads(data->fd[0], NULL, 0, &refs_tmp, 0, NULL);
+ get_remote_heads(data->fd[0], NULL, 0, &refs_tmp, 0,
+NULL, NULL);
data->got_remote_heads = 1;
}
@@ -799,7 +800,7 @@ static int git_transport_push(struct transport *tra
[PATCH 7/7] send-pack: support pushing to a shallow clone
When send-pack receives "shallow" lines from receive-pack, it knows
the other end does not have a complete commit chains. It restrict
itself to the commits that are not cut out by either end to make sure
the result pack is usuable by receive-pack.
The same technique here, using setup_alternate_shallow() and
--shallow-file, might simplify similar code in upload-pack.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/send-pack.c | 7 +--
send-pack.c | 38 +-
send-pack.h | 4 +++-
t/t5537-push-shallow.sh | 18 ++
transport.c | 5 +++--
5 files changed, 62 insertions(+), 10 deletions(-)
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index f0ad0ce..b177120 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -94,6 +94,7 @@ int cmd_send_pack(int argc, const char **argv, const char
*prefix)
int fd[2];
struct child_process *conn;
struct extra_have_objects extra_have;
+ struct extra_have_objects shallow;
struct ref *remote_refs, *local_refs;
int ret;
int helper_status = 0;
@@ -206,8 +207,10 @@ int cmd_send_pack(int argc, const char **argv, const char
*prefix)
}
memset(&extra_have, 0, sizeof(extra_have));
+ memset(&shallow, 0, sizeof(shallow));
- get_remote_heads(fd[0], NULL, 0, &remote_refs, REF_NORMAL, &extra_have,
NULL);
+ get_remote_heads(fd[0], NULL, 0, &remote_refs, REF_NORMAL,
+&extra_have, &shallow);
transport_verify_remote_names(nr_refspecs, refspecs);
@@ -227,7 +230,7 @@ int cmd_send_pack(int argc, const char **argv, const char
*prefix)
set_ref_status_for_push(remote_refs, args.send_mirror,
args.force_update);
- ret = send_pack(&args, fd, conn, remote_refs, &extra_have);
+ ret = send_pack(&args, fd, conn, remote_refs, &extra_have, &shallow);
if (helper_status)
print_helper_status(remote_refs);
diff --git a/send-pack.c b/send-pack.c
index 81d4b1c..08de681 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -27,14 +27,19 @@ static int feed_object(const unsigned char *sha1, int fd,
int negative)
/*
* Make a pack stream and spit it out into file descriptor fd
*/
-static int pack_objects(int fd, struct ref *refs, struct extra_have_objects
*extra, struct send_pack_args *args)
+static int pack_objects(int fd, struct ref *refs,
+ struct extra_have_objects *extra,
+ struct extra_have_objects *extra_shallow,
+ struct send_pack_args *args)
{
/*
* The child becomes pack-objects --revs; we feed
* the revision parameters to it via its stdin and
* let its stdout go back to the other end.
*/
- const char *argv[] = {
+ const char *av[] = {
+ "--shallow-file",
+ NULL,
"pack-objects",
"--all-progress-implied",
"--revs",
@@ -45,10 +50,27 @@ static int pack_objects(int fd, struct ref *refs, struct
extra_have_objects *ext
NULL,
NULL,
};
+ const char **argv;
struct child_process po;
+ static struct lock_file shallow_lock;
+ const char *alternate_shallow_file = NULL;
int i;
- i = 4;
+ if (extra_shallow->nr) {
+ memset(&shallow_lock, 0, sizeof(shallow_lock));
+ /* just to load up .git/shallow if exists */
+ is_repository_shallow();
+ setup_alternate_shallow(&shallow_lock,
+ &alternate_shallow_file,
+ extra_shallow,
+ 0);
+ av[1] = alternate_shallow_file;
+ argv = av;
+ i = 6;
+ } else {
+ argv = &av[2];
+ i = 4;
+ }
if (args->use_thin_pack)
argv[i++] = "--thin";
if (args->use_ofs_delta)
@@ -100,6 +122,10 @@ static int pack_objects(int fd, struct ref *refs, struct
extra_have_objects *ext
if (finish_command(&po))
return -1;
+
+ if (extra_shallow->nr)
+ rollback_lock_file(&shallow_lock);
+
return 0;
}
@@ -176,7 +202,8 @@ static int sideband_demux(int in, int out, void *data)
int send_pack(struct send_pack_args *args,
int fd[], struct child_process *conn,
struct ref *remote_refs,
- struct extra_have_objects *extra_have)
+ struct extra_have_objects *extra_have,
+ struct extra_have_objects *extra_shallow)
{
int in = fd[0];
int out = fd[1];
@@ -294,7 +321,8 @@ int send_pack(struct send_pack_args *args,
}
if (new_refs && cmds_sent) {
- if (pack_objects(out, remote_refs, extra_have, args) < 0) {
+
[PATCH 5/7] fetch-pack: support fetching from a shallow repository
upload-pack already advertises all shallow grafts if server repository
is shallow. This information can be used to add more grafts to the
client if the server sends commit chains down to its graft points.
If the server is shallow, before we receive the pack, we setup a
temporary shallow file that contains both local graft points and the
server's. This stops index-pack from going beyond server's graft
points.
Only server graft points that do not have corresponding SHA-1s in
local repo are added to the temp shallow file because we don't want to
accidentally cut the client history because the server's is
shorter. The client cutting can only happen when --depth is requested.
After index-pack finishes successfully, we write the temporary shallow
down with one exception: unused graft points provided by the server
are removed. We don't want those lying around and suddenly become
active.
Note that in the "shallow -> shallow" case, the server might not have
enough information to find common roots to create an optimum pack. It
might send complete commit chains down to the graft points as a
result. I don't think we can improve this, unless upload-pack somehow
has access to a full repository.
"shallow -> shallow" case only makes sense when the upstream provides
a stable shallow repo (e.g. make a cut every year or so and ask devs
to all move to the new base). If the cloned repos are all based on a
stable (shallow) upstream, the above problem is unlikely to happen.
A side effect of this change is we can now clone from a shallow
repository. And a full repository may automatically become shallow if
you fetch from a shallow repository.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/fetch-pack.c | 6 ++--
commit.h | 8 +++--
fetch-pack.c | 27 +++---
fetch-pack.h | 1 +
shallow.c | 46
t/t5536-fetch-shallow.sh (new +x) | 75 +++
transport.c | 8 +++--
7 files changed, 153 insertions(+), 18 deletions(-)
create mode 100755 t/t5536-fetch-shallow.sh
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 080e599..b89d753 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -37,6 +37,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char
*prefix)
char **pack_lockfile_ptr = NULL;
struct child_process *conn;
struct fetch_pack_args args;
+ struct extra_have_objects shallow;
packet_trace_identity("fetch-pack");
@@ -144,10 +145,11 @@ int cmd_fetch_pack(int argc, const char **argv, const
char *prefix)
args.verbose ? CONNECT_VERBOSE : 0);
}
- get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, NULL);
+ memset(&shallow, 0, sizeof(shallow));
+ get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow);
ref = fetch_pack(&args, fd, conn, ref, dest,
-sought, nr_sought, pack_lockfile_ptr);
+sought, nr_sought, &shallow, pack_lockfile_ptr);
if (pack_lockfile) {
printf("lock %s\n", pack_lockfile);
fflush(stdout);
diff --git a/commit.h b/commit.h
index 678fa20..7faf0e4 100644
--- a/commit.h
+++ b/commit.h
@@ -188,9 +188,13 @@ extern struct commit_list *get_shallow_commits(struct
object_array *heads,
extern void check_shallow_file_for_update(void);
extern void set_alternate_shallow_file(const char *path);
extern void advertise_shallow_grafts(int);
-extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol);
+extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol,
+struct extra_have_objects *extra,
+int remove_unused_grafts);
extern void setup_alternate_shallow(struct lock_file *shallow_lock,
- const char **alternate_shallow_file);
+ const char **alternate_shallow_file,
+ struct extra_have_objects *extra,
+ int rewrite);
int is_descendant_of(struct commit *, struct commit_list *);
int in_merge_bases(struct commit *, struct commit *);
diff --git a/fetch-pack.c b/fetch-pack.c
index dc71a2b..68b95a5 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -311,7 +311,7 @@ static int find_common(struct fetch_pack_args *args,
}
if (is_repository_shallow())
- write_shallow_commits(&req_buf, 1);
+ write_shallow_commits(&req_buf, 1, NULL, 0);
if (args->depth > 0)
packet_buf_write(&req_buf, "deepen %d", args->depth);
packet_buf_flush(&req_buf);
@@ -769,6 +769,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args
*args,
int fd[2],
const struct re
Re: [PATCH v2 07/19] make sure partially read index is not changed
Duy Nguyen writes: > On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer > wrote: >> A partially read index file currently cannot be written to disk. Make >> sure that never happens, by erroring out when a caller tries to change a >> partially read index. The caller is responsible for reading the whole >> index when it's trying to change it later. >> >> Forcing the caller to load the right part of the index file instead of >> re-reading it when changing it, gives a bit of a performance advantage, >> by avoiding to read parts of the index twice. > > If you want to be strict about this, put similar check in > fill_stat_cache_info (used by entry.c), cache_tree_invalidate_path and > convert the code base (maybe except unpack-trees.c, just put a check > in the beginning of unpack_trees()) to use wrapper function to change > ce_flags (some still update ce_flags directly, grep them). Some flags > are in memory only and should be allowed to change in partial index, > some are to be written on disk and should not be allowed. I'm not sure anymore we should even be this strict. A partially read index will never make it to the disk, because write_index checks if it's fully read. I think the check in write_index and read_index_filtered would be enough. The only disadvantage would be that it takes a little longer to catch an error that should never happen in the first place. If it occurs the user has bigger problems than the time that not having caught the error earlier adds to the execution of the command. I also don't see a clean way to add the check to fill_stat_cache_info or cache_tree_invalidate_path, because we would need an additional parameter for the index_state, or at least for index_state->filter_opts, which doesn't do anything but check if the index is only partially loaded. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git counterpart to SVN bugtraq properties?
I'm looking for a specification or guidelines on how a Git client should integrate with bug tracking systems. For SVN, one can use bugtraq-properties [1] to specify e.g. the issue tracker URL or how to parse the bug ID from a commit message. AFAIU, there is nothing comparable for Git [2]? If that's actually the case, is someone interested in working out a similar specification for Git? [1] http://code.google.com/p/tortoisesvn/source/browse/tags/version_1.2.0/doc/issuetrackers.txt [2] http://stackoverflow.com/questions/17545548 -Marc -- 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
[no subject]
Reply to this email: globalloanfu...@aol.com Are You Financially down? And you need a Loan ? Under a clear and understandable terms and condition At 3% Contact Us Now With This Email: globalloanfu...@aol.com (1):Full Names:.. (2):Country/State: (3):Amount Needed: (4):Loan duration: (5)hone Number:... :Job Tittle:.. Regards, Mr Greg Peterson -- 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] t6131 - skip tests if on case-insensitive file system
This test fails on Cygwin where the default system configuration does not support case sensitivity (only case retention), so don't run the test on such systems. Signed-off-by: Mark Levedahl --- t/t6131-pathspec-icase.sh | 6 ++ 1 file changed, 6 insertions(+) diff --git a/t/t6131-pathspec-icase.sh b/t/t6131-pathspec-icase.sh index 3215eef..8d4a7fc 100755 --- a/t/t6131-pathspec-icase.sh +++ b/t/t6131-pathspec-icase.sh @@ -3,6 +3,12 @@ test_description='test case insensitive pathspec limiting' . ./test-lib.sh +if test_have_prereq CASE_INSENSITIVE_FS +then + skip_all='skipping case sensitive tests - case insensitive file system' + test_done +fi + test_expect_success 'create commits with glob characters' ' test_commit bar bar && test_commit bAr bAr && -- 1.8.3.2.0.63 -- 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 counterpart to SVN bugtraq properties?
On Wed, Jul 17, 2013 at 03:03:14PM +0200, Marc Strapetz wrote: > I'm looking for a specification or guidelines on how a Git client should > integrate with bug tracking systems. For SVN, one can use > bugtraq-properties [1] to specify e.g. the issue tracker URL or how to > parse the bug ID from a commit message. AFAIU, there is nothing > comparable for Git [2]? If that's actually the case, is someone > interested in working out a similar specification for Git? > > [1] > http://code.google.com/p/tortoisesvn/source/browse/tags/version_1.2.0/doc/issuetrackers.txt > > [2] http://stackoverflow.com/questions/17545548 The Git way to record the issue ID as a footer in the commit message. See for example [1]. Although I'm not aware of any standard for naming this footer. In terms of recording the URL and other data, I think you'd want a dotfile in the repository (perhaps .bugzilla). This shoudld probably be in the gitconfig format, like .gitmodules. I think "all" it needs is to draw up a spec for the names of keys and format of their values, along with the format of footer(s) identifying issues associated with a commit and to persuade UI developers to support it... ;-) [1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4a88f73f14f6d6c94616538427e1235a6d0a5885 -- 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] t6131 - skip tests if on case-insensitive file system
On Wed, Jul 17, 2013 at 8:22 PM, Mark Levedahl wrote: > This test fails on Cygwin where the default system configuration does not > support case sensitivity (only case retention), so don't run the test on > such systems. Yeah. I knew this when I wrote this test but forgot to put the check in. Thanks. We can re-enable the test later, as it does not really need case-insensitive filesystems. > > Signed-off-by: Mark Levedahl > --- > t/t6131-pathspec-icase.sh | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/t/t6131-pathspec-icase.sh b/t/t6131-pathspec-icase.sh > index 3215eef..8d4a7fc 100755 > --- a/t/t6131-pathspec-icase.sh > +++ b/t/t6131-pathspec-icase.sh > @@ -3,6 +3,12 @@ > test_description='test case insensitive pathspec limiting' > . ./test-lib.sh > > +if test_have_prereq CASE_INSENSITIVE_FS > +then > + skip_all='skipping case sensitive tests - case insensitive file > system' > + test_done > +fi > + > test_expect_success 'create commits with glob characters' ' > test_commit bar bar && > test_commit bAr bAr && > -- > 1.8.3.2.0.63 > > -- > 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 -- 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
[StGit PATCH] Fix dirty index errors when resolving conflicts
The patch 6e8fdc58c786a45d7a63c5edf9c702f1874a7a19 causes StGit to raise "warnings" (actually: errors) in the event that there are changes staged in the index and a refresh is performed without specifying either --index or --force. This is great for preventing an entire class of common mistakes, but is also a giant pain when resolving conflicts after a pull/rebase. Depending on the workflow in use, this may occur with a frequency anywhere between "never" and "mulitple times on every pull". This patch removes the pain by: - Reporting unresolved conflicts *before* complaining about staged changes, since it goes without saying that, when present, these are the main problem. - Not complaining about staged changes if there are no unstaged changes in the working directory, since the presence of --index is immaterial in this case. Signed-off-by: Zane Bitter --- stgit/commands/refresh.py | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/stgit/commands/refresh.py b/stgit/commands/refresh.py index a2bab42..331c18d 100644 --- a/stgit/commands/refresh.py +++ b/stgit/commands/refresh.py @@ -247,18 +247,19 @@ def func(parser, options, args): patch_name = get_patch(stack, options.patch) paths = list_files(stack, patch_name, args, options.index, options.update) -# Make sure the index is clean before performing a full refresh -if not options.index and not options.force: -if not stack.repository.default_index.is_clean(stack.head): -raise common.CmdException( -'The index is dirty. Did you mean --index? To force a full refresh use --force.') - # Make sure there are no conflicts in the files we want to # refresh. if stack.repository.default_index.conflicts() & paths: raise common.CmdException( 'Cannot refresh -- resolve conflicts first') +# Make sure the index is clean before performing a full refresh +if not options.index and not options.force: +if not (stack.repository.default_index.is_clean(stack.head) or +stack.repository.default_iw.worktree_clean()): +raise common.CmdException( +'The index is dirty. Did you mean --index? To force a full refresh use --force.') + # Commit index to temp patch, and absorb it into the target patch. retval, temp_name = make_temp_patch( stack, patch_name, paths, temp_index = path_limiting) -- 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
maintainer question: config http..* patch administrivia
I have pondered these items: On Jul 12, 2013, at 11:48, Junio C Hamano wrote: Perhaps we should fix it as a preparatory patch (1/2) before the main "feature addition" patch. On Jul 12, 2013, at 11:52, Junio C Hamano wrote: Subject: [PATCH] http.c: fix parsing of http.sslCertPasswordProtected variable On Jul 14, 2013, at 21:02, Junio C Hamano wrote: Assuming that Aaron and Peff's enhancement will not be a backward incompatible update, my preference is to take the posted matching semantics as-is (you may have some other changes that does not change the "strictly textual match" semantics). And in response I have previously sent out a combined v5 patch that has: 0001: your preparatory http.sslCertPasswordProtected patch 0002: logically related GIT_SSL_CERT_PASSWORD_PROTECTED patch 0003: textual matching http..* patch 0004: url normalization matching http..* 0005: test for url normalization function However, upon further consideration (I noticed that the preparatory patch and v4 of the textual matching patch made their way into pu), perhaps it would be more convenient for you if I re-released the following patch series: [PATCH v5]: config: support http..* settings - (1) textual matching * contains 0001 the same preparatory http.sslCertPasswordProtected * contains 0002 the same v5 textual matching http..* patch [PATCH v2]: config: support http..* settings - (2) url normalization * contains 0001 url normalization matching with feedback updates * contains 0002 url normalization test [PATCH v1]: config: support http..* settings - (3) any user matching * contains 0001 a new patch that extends (2) to include any user matching And drop the GIT_SSL_CERT_PASSWORD_PROTECTED patch as while it's logically related to the http.sslCertPasswordProtected patch it's not logistically related since independent areas of the file are touched and it could be successfully applied before or after the other patches. It can go together with any forthcoming enable-only fix for GIT_SSL_CERT_PASSWORD_PROTECTED and other such environment variables or just be dropped entirely. I do not, however, wish to create any additional maintainer work, so wanted to check with you before sending out any of the reissues. -- 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] Add the NO_SENTINEL build variable
Ramsay Jones writes: > diff --git a/git-compat-util.h b/git-compat-util.h > index 9f1eaca..e846e01 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -300,6 +300,12 @@ extern char *gitbasename(char *); > #endif > #endif > > +#if defined(__GNUC__) && !defined(NO_SENTINEL) > +#define SENTINEL(n) __attribute__((sentinel(n))) > +#else > +#define SENTINEL(n) > +#endif Allowing callers to use __attribute__((sentinel(1)), while it may be a good enhancement, does not belong to "some versions of GCC do not know what to do with the sentinel attribute". Making this change in a separate patch on top would be cleaner. Also do we know what version of GCC started supporting this attribute? http://gcc.gnu.org/gcc-4.0/changes.html mentions it in "New Languages and Language specific improvements" section, but the page also says "The latest release in the 4.0 release series is GCC 4.0.4", so it is not clear if 4.0 had it, or it was added somewhere between 4.0 and 4.0.4 to me. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/3] Switch German translation to G+E
This is a resend of v3 because at least one patch was damaged last time for whatever reason. Ralf Thielow (3): l10n: de.po: switch from pure German to German+English (part 1) l10n: de.po: switch from pure German to German+English (part 2) l10n: de.po: switch from pure German to German+English (part 3) po/de.po | 1831 +++--- 1 file changed, 909 insertions(+), 922 deletions(-) -- 1.8.2.1230.g519726a -- 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] Add the NO_SENTINEL build variable
Junio C Hamano writes: > Also do we know what version of GCC started supporting this > attribute? http://gcc.gnu.org/gcc-4.0/changes.html mentions it in > "New Languages and Language specific improvements" section, but the > page also says "The latest release in the 4.0 release series is GCC > 4.0.4", so it is not clear if 4.0 had it, or it was added somewhere > between 4.0 and 4.0.4 to me. Generally, gcc doesn't get new features added within the same minor version, only bug fixes. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- 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 0/2] git diff -q option removal
Stefan Beller writes:
> The changes in the following patch are in diff_no_index.c, but the
> diff_no_index(...) is called from cmd_diff, which is in builtin/diff.c
> That cmd_diff is actually called from git.c having the
> { "diff", cmd_diff }, entry in handle_internal_command.
>
> My question now is this: Why is the builtin/diff.c relying on stuff
> outside of builtin/ ? Wouldn't it be better to move all these files
> (such as diff_no_index.c) into the builtin folder as well?
Builtins link all sorts of stuff from outside, e.g. diff.c and
diffcore-*.c at the toplevel. I do not see diff_no_index.c is any
different, so I am probably not understanding your question.
> Regarding the removal of the -q option, I tried it in the second patch.
> Is it as easy as that, or am I missing the point?
>
> The first patch doesn't change the behavior, so I'd assume it's safe to
> apply it to origin/sb/misc-fixes, whereas the second patch will make
> git diff complain about the -q option, so I'd assume it would wait for the
> next major release?
>
> Before:
> touch actual_file
> git diff -q actual_file no_file
> error: Could not access 'no_file'
Hmm, do you really get that error message? I think you would get
fatal: ambiguous argument 'no_file': unknown revision or path not in the
working tree.
> echo $?
> 1
The command line parsing infrastructure has changed vastly since
"show-diff" days (see below for a history lesson); I think your
"Before" should read more like this
git diff -q -- actual_file no_file
and it should not show removal of no_file in its output. E.g. in
git.git
$ git reset --hard
$ rm COPYING
$ git diff -q -- COPYING
should show nothing.
I personally think "-q" no longer makes sense in today's codebase,
but I am not convinced that removal of '-q' from the proper "git
diff-files" and the "git diff --no-index" (aka "I am too lazy to
teach our diff enhancement to other people's diff implementations,
so let's throw in a "files do not have to be tracked in Git
repository at all" mode") is the right direction to go.
The "-q" option is a remnant from the "show-diff" command, the
precursor of today's "git diff-files" (back then, we didn't even
have "git" potty. The user literally typed "show-diff", not "git
show-diff").
ca2a0798 ([PATCH] Add "-q" option to show-diff.c, 2005-04-15) added
that option. Back then, we did not have pathspec matching, and we
iterated over command line arguments, and required all of them exist
as filesystem entities. "-q" was a way to defeat that "you name a
file, it must exist in the working tree" safety, and also at the
same time not give output for such a file that was removed from the
working tree.
These days, the former "safety" is enforced by the generalized
revision parser ("is it a path or is it a rev?") code and the "--"
delimiter on the command line is the way to defeat it. The latter
is done by giving a filtering specification that lack D to the
"--diff-filter".
If we wanted to make "-q" follow the spirit of its original addition
to "show-diff" again, we could internally add a diff-filter when the
"-q" option is parsed.
"git diff -q ..." is "git diff --diff-filter=ACMRTUB ...", and "git
diff -q --diff-filter=AD" is "git diff --diff-filter=A". That would
let us remove the special case for SILENT_ON_REMOVED, and also make
"-q" work across various commands in the "diff" family. It might
even make it work for "diff --no-index", but I didn't bother to
check.
> After:
> touch actual_file
> git diff -q actual_file no_file
> fatal: invalid diff option/value: -q
> echo $?
> 128
>
> Thanks,
> Stefan
>
> Stefan Beller (2):
> diff --no-index: remove nonfunctional "-q" handling
> git diff: Remove -q option to stay silent on missing files.
>
> Documentation/git-diff-files.txt | 6 +-
> diff-no-index.c | 5 -
> 2 files changed, 1 insertion(+), 10 deletions(-)
--
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] request-pull: improve error message for invalid revision args
Dirk Wallenstein writes: > When an invalid revision is specified, the error message is: > > fatal: Needed a single revision > > This is misleading because, you might think there is something wrong > with the command line as a whole. > > Now the user gets a more meaningful error message, showing the invalid > revision. > > Signed-off-by: Dirk Wallenstein > --- > > Notes: > I assume, it is not worth the trouble to even try to change the message > from > rev-parse for this. People might parse the messages, which is probably > why > this message still exists. You are right---such a change will break existing scripts, so it is not just "not worth the trouble" but is actively wrong to change the error message. > git-request-pull.sh | 14 -- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/git-request-pull.sh b/git-request-pull.sh > index d566015..f38f0f9 100755 > --- a/git-request-pull.sh > +++ b/git-request-pull.sh > @@ -51,8 +51,18 @@ fi > tag_name=$(git describe --exact "$head^0" 2>/dev/null) > > test -n "$base" && test -n "$url" || usage > -baserev=$(git rev-parse --verify "$base"^0) && > -headrev=$(git rev-parse --verify "$head"^0) || exit > + > +baserev=$(git rev-parse --verify "$base"^0 2>/dev/null) Use "--quiet" instead? > +if test -z "$baserev" > +then > +die "fatal: Not a valid revision: $base" > +fi > + > +headrev=$(git rev-parse --verify "$head"^0 2>/dev/null) > +if test -z "$headrev" > +then > +die "fatal: Not a valid revision: $head" > +fi > > merge_base=$(git merge-base $baserev $headrev) || > die "fatal: No commits in common between $base and $head" -- 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/7] remote.c: add command line option parser for --lockref
John Keeping writes:
> On Tue, Jul 09, 2013 at 12:53:27PM -0700, Junio C Hamano wrote:
>> diff --git a/remote.c b/remote.c
>> index 81bc876..e9b423a 100644
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -1938,3 +1938,62 @@ struct ref *get_stale_heads(struct refspec *refs, int
>> ref_count, struct ref *fet
>> string_list_clear(&ref_names, 0);
>> return stale_refs;
>> }
>> +
>> +/*
>> + * Lockref aka CAS
>> + */
>> +void clear_cas_option(struct push_cas_option *cas)
>> +{
>> +int i;
>> +
>> +for (i = 0; i < cas->nr; i++)
>> +free(cas->entry->refname);
>
> Should this be
>
> free(cas->entry[i]->refname);
>
> ?
Yes, I think so.
--
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/7] remote.c: add command line option parser for --lockref
John Keeping writes:
> On Tue, Jul 09, 2013 at 12:53:27PM -0700, Junio C Hamano wrote:
>> diff --git a/remote.c b/remote.c
>> index 81bc876..e9b423a 100644
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -1938,3 +1938,62 @@ struct ref *get_stale_heads(struct refspec *refs, int
>> ref_count, struct ref *fet
>> string_list_clear(&ref_names, 0);
>> return stale_refs;
>> }
>> +
>> +/*
>> + * Lockref aka CAS
>> + */
>> +void clear_cas_option(struct push_cas_option *cas)
>> +{
>> +int i;
>> +
>> +for (i = 0; i < cas->nr; i++)
>> +free(cas->entry->refname);
>
> Should this be
>
> free(cas->entry[i]->refname);
>
> ?
Yes, more like "free(cas->entry[i].refname)".
Thanks for spotting.
>
>> +free(cas->entry);
>> +memset(cas, 0, sizeof(*cas));
>> +}
--
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: maintainer question: config http..* patch administrivia
"Kyle J. McKay" writes: > However, upon further consideration (I noticed that the preparatory > patch and v4 of the textual matching patch made their way into pu), Do not read too much into being in 'pu'. They are there primarily so that I won't forget what is in-flight and can be replaced or dropped. > perhaps it would be more convenient for you if I re-released the > following patch series: Sorry, but I do not understand. What do you mean by having these three patch series? Are they "There are three possibilities, pick one and discard the other two"? If so, then as the maintainer, I personally would not care ;-), as they would be commented upon, and I won't pick any of them up until there is one list favorite. If "These build on top of each other in this order", then it is easier for me to manage if they were in a single series. If f1ff763a (http.c: fix parsing of http.sslCertPasswordProtected variable, 2013-07-12) is already solid and need no further tweaking, it would be wise not to include the patch in any of your reroll in any case. Instead, just build your series on top of that commit, and make it clear that the series is meant to apply on top of that commit in the cover letter [0/n] of the series, or after "---" line of the first patch [1/n] in the series. As to allowing GIT_SSL_CERT_PASSWORD_PROTECTED=no, I agree that it does not belong to the http.*. configuration series, so making it an independent patch, perhaps on top of f1ff763a, would make sense. If the other patches have any textual dependency, you can make it [1/n] of your series and we can treat it just like f1ff763a, that is, make sure it is solid regardless of the rest of the series, and make it advance before the remainder of the series, so that we can still replace http.*. implementation on top of these two freely. > > [PATCH v5]: config: support http..* settings - (1) textual matching > > * contains 0001 the same preparatory http.sslCertPasswordProtected > * contains 0002 the same v5 textual matching http..* patch > > > [PATCH v2]: config: support http..* settings - (2) url > normalization > > * contains 0001 url normalization matching with feedback updates > * contains 0002 url normalization test > > > [PATCH v1]: config: support http..* settings - (3) any user > matching > > * contains 0001 a new patch that extends (2) to include any user > matching > > > And drop the GIT_SSL_CERT_PASSWORD_PROTECTED patch as while it's > logically related to the http.sslCertPasswordProtected patch it's not > logistically related since independent areas of the file are touched > and it could be successfully applied before or after the other > patches. It can go together with any forthcoming enable-only fix for > GIT_SSL_CERT_PASSWORD_PROTECTED and other such environment variables > or just be dropped entirely. > > I do not, however, wish to create any additional maintainer work, so > wanted to check with you before sending out any of the reissues. -- 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] request-pull: improve error message for invalid revision args
Currently, when an invalid revision is specified, the error message is: fatal: Needed a single revision This is misleading because, you might think there is something wrong with the command line as a whole. Now the user gets a more meaningful error message, showing the invalid revision. Signed-off-by: Dirk Wallenstein --- On Wed, Jul 17, 2013 at 10:06:21AM -0700, Junio C Hamano wrote: > Dirk Wallenstein writes: > > +baserev=$(git rev-parse --verify "$base"^0 2>/dev/null) > > Use "--quiet" instead? Oh, of course. git-request-pull.sh | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/git-request-pull.sh b/git-request-pull.sh index d566015..ebf1269 100755 --- a/git-request-pull.sh +++ b/git-request-pull.sh @@ -51,8 +51,18 @@ fi tag_name=$(git describe --exact "$head^0" 2>/dev/null) test -n "$base" && test -n "$url" || usage -baserev=$(git rev-parse --verify "$base"^0) && -headrev=$(git rev-parse --verify "$head"^0) || exit + +baserev=$(git rev-parse --verify --quiet "$base"^0) +if test -z "$baserev" +then +die "fatal: Not a valid revision: $base" +fi + +headrev=$(git rev-parse --verify --quiet "$head"^0) +if test -z "$headrev" +then +die "fatal: Not a valid revision: $head" +fi merge_base=$(git merge-base $baserev $headrev) || die "fatal: No commits in common between $base and $head" -- 1.8.3.3.2.g85103ba -- 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 0/2] git diff -q option removal
On Wed, Jul 17, 2013 at 10:04 AM, Junio C Hamano wrote:
> If we wanted to make "-q" follow the spirit of its original addition
> to "show-diff" again, we could internally add a diff-filter when the
> "-q" option is parsed.
Having said all that, I do not mean to advocate to retain "-q". Most
likely nobody uses it, and "-q" is grossly misnamed ("why is it so
special to be "quiet" only for removals?"). As long as we mention its
removal in the release notes (and possibly tell those miniscule
minority that wants to ignore deleted files to use --diff-filter
instead), we should be OK.
Independently, we might want to enhance --diff-filter parser to make
it easier to say "I want everything but D", perhaps use lowercase
letter to subtract from what have been specified so far (or if there
is no uppercase letter, start from "everything"), so that we can say
--diff-filter=d
or something.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: maintainer question: config http..* patch administrivia
On Jul 17, 2013, at 10:35, Junio C Hamano wrote: "Kyle J. McKay" writes: perhaps it would be more convenient for you if I re-released the following patch series: If "These build on top of each other in this order", then it is easier for me to manage if they were in a single series. Yup. They build on one another so each requires the previous. Will keep them in a single series. If f1ff763a (http.c: fix parsing of http.sslCertPasswordProtected variable, 2013-07-12) is already solid and need no further tweaking, it would be wise not to include the patch in any of your reroll in any case. Instead, just build your series on top of that commit, and make it clear that the series is meant to apply on top of that commit in the cover letter [0/n] of the series, or after "---" line of the first patch [1/n] in the series. Got it. Will do. f1ff763a looks solid to me: Reviewed-by: Kyle J. McKay And earlier: On Jul 12, 2013, at 12:05, Jonathan Nieder wrote: Subject: Re: [PATCH] http.c: fix parsing of http.sslCertPasswordProtected variable this change looks good. Reviewed-by: Jonathan Nieder so I will drop it from any future series and just refer to it in the cover letter as you suggest. As to allowing GIT_SSL_CERT_PASSWORD_PROTECTED=no, I agree that it does not belong to the http.*. configuration series Agreed. Will not include it in any future http.*. series. If the other patches have any textual dependency, you can make it [1/n] of your series and we can treat it just like f1ff763a, that is, make sure it is solid regardless of the rest of the series, and make it advance before the remainder of the series, so that we can still replace http.*. implementation on top of these two freely. Got it. Will do that and keep it in a single series. Thanks for the direction, Kyle -- 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 1/2] git_mkstemps: correctly test return value of open()
Thomas Rast writes: > Thomas Rast writes: > >> From: "Dale R. Worley" >> >> open() returns -1 on failure, and indeed 0 is a possible success value >> if the user closed stdin in our process. Fix the test. >> >> Signed-off-by: Thomas Rast > > I see you have this in 'pu' without Dale's signoff. I'm guessing > (IANAL) that it's too small to be copyrighted and anyway there is only > way to fix it, but maybe Dale can "sign off" just to be safe, anyway? Yup, that is a good idea. Thanks. > >> wrapper.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/wrapper.c b/wrapper.c >> index dd7ecbb..6a015de 100644 >> --- a/wrapper.c >> +++ b/wrapper.c >> @@ -322,7 +322,7 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int >> mode) >> template[5] = letters[v % num_letters]; v /= num_letters; >> >> fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode); >> -if (fd > 0) >> +if (fd >= 0) >> return fd; >> /* >> * Fatal error (EPERM, ENOSPC etc). -- 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] t3032 - make compatible with systems using \r\n as a line ending
Mark Levedahl writes:
> Subtests 6, 7, and 9 rely test that merge-recursive correctly
> ignores whitespace when so directed. These tests create and test for
> lines ending in \r\n, but as this is a valid line separator on Windows,
> convert such lines in the output to avoid confusion by line-oriented
> grep.
>
> Signed-off-by: Mark Levedahl
> ---
> Sorry, forgot to copy Jonathan...
Sounds sensible. Thanks.
>
> t/t3032-merge-recursive-options.sh | 22 +-
> t/test-lib-functions.sh| 4
> 2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/t/t3032-merge-recursive-options.sh
> b/t/t3032-merge-recursive-options.sh
> index 2b17311..41ba184 100755
> --- a/t/t3032-merge-recursive-options.sh
> +++ b/t/t3032-merge-recursive-options.sh
> @@ -125,13 +125,14 @@ test_expect_success '-Xignore-space-change makes
> cherry-pick succeed' '
> '
>
> test_expect_success '--ignore-space-change: our w/s-only change wins' '
> - q_to_cr <<-\EOF >expected &&
> + cat <<-\EOF >expected &&
> justice and holiness and is the nurse of his age and theQ
> EOF
>
> git read-tree --reset -u HEAD &&
> git merge-recursive --ignore-space-change HEAD^ -- HEAD remote &&
> - grep "justice and holiness" text.txt >actual &&
> + cr_to_q text.txt+ &&
> + grep "justice and holiness" text.txt+ >actual &&
> test_cmp expected actual
> '
>
> @@ -150,14 +151,15 @@ test_expect_success '--ignore-space-change: does not
> ignore new spaces' '
> cat <<-\EOF >expected1 &&
> Well said, Cephalus, I replied; but as con cerning justice, what is
> EOF
> - q_to_cr <<-\EOF >expected2 &&
> + cat <<-\EOF >expected2 &&
> un intentionally; and when he departs to the world below he is not inQ
> EOF
>
> git read-tree --reset -u HEAD &&
> git merge-recursive --ignore-space-change HEAD^ -- HEAD remote &&
> - grep "Well said" text.txt >actual1 &&
> - grep "when he departs" text.txt >actual2 &&
> + cr_to_q text.txt+
> + grep "Well said" text.txt+ >actual1 &&
> + grep "when he departs" text.txt+ >actual2 &&
> test_cmp expected1 actual1 &&
> test_cmp expected2 actual2
> '
> @@ -174,18 +176,19 @@ test_expect_success '--ignore-all-space drops their new
> spaces' '
> '
>
> test_expect_success '--ignore-all-space keeps our new spaces' '
> - q_to_cr <<-\EOF >expected &&
> + cat <<-\EOF >expected &&
> un intentionally; and when he departs to the world below he is not inQ
> EOF
>
> git read-tree --reset -u HEAD &&
> git merge-recursive --ignore-all-space HEAD^ -- HEAD remote &&
> - grep "when he departs" text.txt >actual &&
> + cr_to_q text.txt+ &&
> + grep "when he departs" text.txt+ >actual &&
> test_cmp expected actual
> '
>
> test_expect_success '--ignore-space-at-eol' '
> - q_to_cr <<-\EOF >expected &&
> + cat <<-\EOF >expected &&
> <<< HEAD
> is not in his right mind; ought I to give them back to him? No oneQ
> ===
> @@ -196,7 +199,8 @@ test_expect_success '--ignore-space-at-eol' '
> git read-tree --reset -u HEAD &&
> test_must_fail git merge-recursive --ignore-space-at-eol \
>HEAD^ -- HEAD remote &&
> - conflict_hunks text.txt >actual &&
> + cr_to_q text.txt+ &&
> + conflict_hunks text.txt+ >actual &&
> test_cmp expected actual
> '
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index a7e9aac..aa8e38f 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -87,6 +87,10 @@ q_to_cr () {
> tr Q '\015'
> }
>
> +cr_to_q () {
> + tr '\015' Q
> +}
> +
> q_to_tab () {
> tr Q '\011'
> }
--
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] t6131 - skip tests if on case-insensitive file system
Duy Nguyen writes: > On Wed, Jul 17, 2013 at 8:22 PM, Mark Levedahl wrote: >> This test fails on Cygwin where the default system configuration does not >> support case sensitivity (only case retention), so don't run the test on >> such systems. > > Yeah. I knew this when I wrote this test but forgot to put the check > in. Thanks. We can re-enable the test later, as it does not really > need case-insensitive filesystems. Yeah, I tend to agree. You do not need to create case-colliding paths for the purpose of :(icase) tests. > >> >> Signed-off-by: Mark Levedahl >> --- >> t/t6131-pathspec-icase.sh | 6 ++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/t/t6131-pathspec-icase.sh b/t/t6131-pathspec-icase.sh >> index 3215eef..8d4a7fc 100755 >> --- a/t/t6131-pathspec-icase.sh >> +++ b/t/t6131-pathspec-icase.sh >> @@ -3,6 +3,12 @@ >> test_description='test case insensitive pathspec limiting' >> . ./test-lib.sh >> >> +if test_have_prereq CASE_INSENSITIVE_FS >> +then >> + skip_all='skipping case sensitive tests - case insensitive file >> system' >> + test_done >> +fi >> + >> test_expect_success 'create commits with glob characters' ' >> test_commit bar bar && >> test_commit bAr bAr && >> -- >> 1.8.3.2.0.63 >> >> -- >> 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 > -- > 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] .mailmap: René Scharfe has a new email address
Signed-off-by: René Scharfe --- I failed to log on to the dyn.com website in time and lost my old free DNS entry. :-/ .mailmap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap index 345cce6..5c16adf 100644 --- a/.mailmap +++ b/.mailmap @@ -78,7 +78,7 @@ Petr Baudis Philippe Bruhat Ralf Thielow Ramsay Allan Jones -René Scharfe +René Scharfe Robert Fitzsimons Robert Zeh Sam Vilain -- 1.8.3.3 -- 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 0/2] git diff -q option removal
On 07/17/2013 07:04 PM, Junio C Hamano wrote: > Builtins link all sorts of stuff from outside, e.g. diff.c and > diffcore-*.c at the toplevel. I do not see diff_no_index.c is any > different, so I am probably not understanding your question. Thanks for the explanation. I am not yet very used to gits code structure. So I sometimes think of 'how would I structure things', so I was confused of things in builtin using some parts outside of it. Maybe that folder raised to much expectations for me to be 'the real' core, whereas the files outside, i.e. those files in the top level directory, are just there for other things or scripts. This understanding of the structure seems obviously wrong now. Thanks for clarification. > > Hmm, do you really get that error message? I think you would get > > fatal: ambiguous argument 'no_file': unknown revision or path not in the > working tree. > >> echo $? >> 1 Ok here we go (using current origin/master 9c3c367): cd $(mktemp -d) echo "test" > actual_file git diff actual_file no_file # error: Could not access 'no_file' echo $? 1 ## I get the same message as well, if I'm using -- or not. ## also the -q doesn't make a change git init git diff -q -- actual_file no_file echo $? # 0 git diff -- actual_file no_file echo $? # 0 git diff actual_file no_file # fatal: no_file: no such path in the working tree. # Use 'git -- ...' to specify paths that do not exist locally. echo $? # 128 git diff -q actual_file no_file # fatal: no_file: no such path in the working tree. # Use 'git -- ...' to specify paths that do not exist locally. echo $? 128 So apparently git diff behaves differently if not in a repo, which is what I tested. > > The command line parsing infrastructure has changed vastly since > "show-diff" days (see below for a history lesson); A very interesting read, much appreciated. :) > > If we wanted to make "-q" follow the spirit of its original addition > to "show-diff" again, we could internally add a diff-filter when the > "-q" option is parsed. > I'm rather new to the project, so my opinion may not have much weight, I'll state it anyway: Keeping backwards compatibility is really hard, because you need the knowledge of such history lessons as read above, to understand what should happen, like having an intuitive feeling about such parameters. Hence maintaining/evolving the project will become harder and harder (specially for newcomers). So having one and only one way to achieve the desired output, which fits into the greater structure as it's the case with --diff-filter is easier to remember for both the user and developers. Hence I think keeping the -q option would only make sense for the plumber layer, because there the explicit promise was given to not change stuff every other release. Stefan signature.asc Description: OpenPGP digital signature
[PATCH] .mailmap: Combine more (email, name) to individual persons
I got more responses from people regarding the .mailmap file. All added persons gave permission to add them to the .mailmap file. Signed-off-by: Stefan Beller --- .mailmap | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.mailmap b/.mailmap index 9430d14..df8898f 100644 --- a/.mailmap +++ b/.mailmap @@ -28,8 +28,7 @@ Chris Shoemaker Chris Wright Csaba Henk Dan Johnson -Dana L. How -Dana L. How +Dana L. How Daniel Barkalow David Brown David D. Kilzer @@ -52,6 +51,7 @@ Frédéric Heitzmann Garry Dolley Greg Price Greg Price +Heiko Voigt H. Merijn Brand H.Merijn Brand H. Peter Anvin H. Peter Anvin @@ -74,7 +74,8 @@ Johannes Schindelin Johannes Sixt Johannes Sixt Johannes Sixt -Jon Loeliger +Jon Loeliger +Jon Loeliger Jon Seymour Jonathan Nieder Jonathan del Strother @@ -175,10 +176,13 @@ Sean Estabrooks Sebastian Schuberth Seth Falcon Shawn O. Pearce +Simon Hausmann +Simon Hausmann Stefan Naewe Stefan Naewe Stefan Sperling Stephen Boyd +Steven Drake Steven Grimm Steven Walter Steven Walter -- 1.8.3.3.754.g9c3c367.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] apply, entry: speak of submodules instead of subprojects
Thomas Rast writes: >> But a 'git grep "corrupt patch for sub"' shows some files in the po >> directory still containing that string on current master. Shouldn't >> they be changed too or is this just a sign of me not understanding >> the translation process? > > I haven't checked any guides, but I imagine that the resulting manual > translation update is what prompts the translators to also check if they > need to adapt the message. I would suggest to leave the *.po files as they are. I suspect that some translations might already use a translation of "submodule", and others use "subproject". All of them should make sure their translation is appropriate when *.pot is updated. -- 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: [StGit PATCH] Fix dirty index errors when resolving conflicts
> -Original Message- > From: Zane Bitter [mailto:zbit...@redhat.com] > Sent: Wednesday, July 17, 2013 6:57 AM > To: git@vger.kernel.org > Cc: Keller, Jacob E; Waskiewicz Jr, Peter P; catalin.mari...@gmail.com > Subject: [StGit PATCH] Fix dirty index errors when resolving conflicts > > The patch 6e8fdc58c786a45d7a63c5edf9c702f1874a7a19 causes StGit > to raise > "warnings" (actually: errors) in the event that there are changes staged in > the index and a refresh is performed without specifying either --index or > --force. This is great for preventing an entire class of common mistakes, > but is also a giant pain when resolving conflicts after a pull/rebase. > Depending on the workflow in use, this may occur with a frequency > anywhere > between "never" and "mulitple times on every pull". > > This patch removes the pain by: > - Reporting unresolved conflicts *before* complaining about staged >changes, since it goes without saying that, when present, these are the >main problem. > - Not complaining about staged changes if there are no unstaged changes > in >the working directory, since the presence of --index is immaterial in >this case. > > Signed-off-by: Zane Bitter > --- > stgit/commands/refresh.py | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/stgit/commands/refresh.py b/stgit/commands/refresh.py > index a2bab42..331c18d 100644 > --- a/stgit/commands/refresh.py > +++ b/stgit/commands/refresh.py > @@ -247,18 +247,19 @@ def func(parser, options, args): > patch_name = get_patch(stack, options.patch) > paths = list_files(stack, patch_name, args, options.index, > options.update) > > -# Make sure the index is clean before performing a full refresh > -if not options.index and not options.force: > -if not stack.repository.default_index.is_clean(stack.head): > -raise common.CmdException( > -'The index is dirty. Did you mean --index? To force a full > refresh > use --force.') > - > # Make sure there are no conflicts in the files we want to > # refresh. > if stack.repository.default_index.conflicts() & paths: > raise common.CmdException( > 'Cannot refresh -- resolve conflicts first') > > +# Make sure the index is clean before performing a full refresh > +if not options.index and not options.force: > +if not (stack.repository.default_index.is_clean(stack.head) or > +stack.repository.default_iw.worktree_clean()): > +raise common.CmdException( > +'The index is dirty. Did you mean --index? To force a full > refresh > use --force.') > + > # Commit index to temp patch, and absorb it into the target patch. > retval, temp_name = make_temp_patch( > stack, patch_name, paths, temp_index = path_limiting) ACK. This looks great. Thanks for noticing! - Jake
Re: [StGit PATCH] Fix dirty index errors when resolving conflicts
On Wed, 2013-07-17 at 15:57 +0200, Zane Bitter wrote:
> The patch 6e8fdc58c786a45d7a63c5edf9c702f1874a7a19 causes StGit to raise
> "warnings" (actually: errors) in the event that there are changes staged in
> the index and a refresh is performed without specifying either --index or
> --force. This is great for preventing an entire class of common mistakes,
> but is also a giant pain when resolving conflicts after a pull/rebase.
> Depending on the workflow in use, this may occur with a frequency anywhere
> between "never" and "mulitple times on every pull".
I actually had someone else mention this problem to me last week. This
looks good, I'll be applying this later today.
Cheers,
-PJ
>
> This patch removes the pain by:
> - Reporting unresolved conflicts *before* complaining about staged
>changes, since it goes without saying that, when present, these are the
>main problem.
> - Not complaining about staged changes if there are no unstaged changes in
>the working directory, since the presence of --index is immaterial in
>this case.
>
> Signed-off-by: Zane Bitter
> ---
> stgit/commands/refresh.py | 13 +++--
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/stgit/commands/refresh.py b/stgit/commands/refresh.py
> index a2bab42..331c18d 100644
> --- a/stgit/commands/refresh.py
> +++ b/stgit/commands/refresh.py
> @@ -247,18 +247,19 @@ def func(parser, options, args):
> patch_name = get_patch(stack, options.patch)
> paths = list_files(stack, patch_name, args, options.index,
> options.update)
>
> -# Make sure the index is clean before performing a full refresh
> -if not options.index and not options.force:
> -if not stack.repository.default_index.is_clean(stack.head):
> -raise common.CmdException(
> -'The index is dirty. Did you mean --index? To force a full
> refresh use --force.')
> -
> # Make sure there are no conflicts in the files we want to
> # refresh.
> if stack.repository.default_index.conflicts() & paths:
> raise common.CmdException(
> 'Cannot refresh -- resolve conflicts first')
>
> +# Make sure the index is clean before performing a full refresh
> +if not options.index and not options.force:
> +if not (stack.repository.default_index.is_clean(stack.head) or
> +stack.repository.default_iw.worktree_clean()):
> +raise common.CmdException(
> +'The index is dirty. Did you mean --index? To force a full
> refresh use --force.')
> +
> # Commit index to temp patch, and absorb it into the target patch.
> retval, temp_name = make_temp_patch(
> stack, patch_name, paths, temp_index = path_limiting)
>
N�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf
[PATCH 0/6] fix blame -L regression; add tests
This series fixes a regression in "blame -L X,-N", adds blame -L tests, and makes minor documentation adjustments. The tests, in particular, were motivated by the desire to revisit and continue working on [1] which extends git-blame to accept multiple -L's. That topic will need to extend blame -L tests, of which there were essentially none. Patches [2/6] (modernize style) and [3/6] (add blame -L tests) are intentionally independent of the "git log -L" topic (from earlier this year) to which the other patches are related. This independence should allow these two patches to graduate at their own pace without being tied to "git log -L". [1]: http://thread.gmane.org/gmane.comp.version-control.git/229755/ Eric Sunshine (6): line-range: fix "blame -L X,-N" regression t8001/t8002 (blame): modernize style t8001/t8002 (blame): add blame -L tests t8001/t8002 (blame): add blame -L :funcname tests blame-options.txt: place each -L option variation on its own line blame-options.txt: explain that -L and are optional Documentation/blame-options.txt | 10 +- line-range.c| 7 + t/annotate-tests.sh | 363 t/t8001-annotate.sh | 6 +- t/t8002-blame.sh| 12 +- 5 files changed, 283 insertions(+), 115 deletions(-) -- 1.8.3.3.1016.g4f0baba -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] t8001/t8002 (blame): add blame -L :funcname tests
git-blame inherited "-L :funcname" support when "-L :funcname:file" was
implemented for git-log. Add tests.
Signed-off-by: Eric Sunshine
---
t/annotate-tests.sh | 48 ++--
1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index b6a7478..0bfee00 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -3,17 +3,19 @@
check_count () {
head= &&
+ file='file' &&
options= &&
while :
do
case "$1" in
-h) head="$2"; shift; shift ;;
+ -f) file="$2"; shift; shift ;;
-*) options="$options $1"; shift ;;
*) break ;;
esac
done &&
- echo "$PROG $options file $head" >&4 &&
- $PROG $options file $head >actual &&
+ echo "$PROG $options $file $head" >&4 &&
+ $PROG $options $file $head >actual &&
perl -e '
my %expect = (@ARGV);
my %count = map { $_ => 0 } keys %expect;
@@ -231,6 +233,48 @@ test_expect_success 'blame -L ,Y (Y > nlines)' '
test_must_fail $PROG -L,12345 file
'
+test_expect_success 'setup -L :regex' '
+ tr Q "\\t" >hello.c <<-\EOF &&
+ int main(int argc, const char *argv[])
+ {
+ Qputs("hello");
+ }
+ EOF
+ git add hello.c &&
+ GIT_AUTHOR_NAME="F" GIT_AUTHOR_EMAIL="f...@test.git" \
+ git commit -m "hello" &&
+
+ mv hello.c hello.orig &&
+ sed -e "/}/i\\
+ Qputs(\"goodbye\");" hello.c &&
+ GIT_AUTHOR_NAME="G" GIT_AUTHOR_EMAIL="g...@test.git" \
+ git commit -a -m "goodbye" &&
+
+ mv hello.c hello.orig &&
+ echo "#include " >hello.c &&
+ cat hello.orig >>hello.c &&
+ tr Q "\\t" >>hello.c <<-\EOF
+ void mail()
+ {
+ Qputs("mail");
+ }
+ EOF
+ GIT_AUTHOR_NAME="H" GIT_AUTHOR_EMAIL="h...@test.git" \
+ git commit -a -m "mail"
+'
+
+test_expect_success 'blame -L :literal' '
+ check_count -f hello.c -L:main F 4 G 1
+'
+
+test_expect_success 'blame -L :regex' '
+ check_count -f hello.c "-L:m[a-z][a-z]l" H 4
+'
+
+test_expect_success 'blame -L :nomatch' '
+ test_must_fail $PROG -L:nomatch hello.c
+'
+
test_expect_success 'blame -L bogus' '
test_must_fail $PROG -L file &&
test_must_fail $PROG -L1,+ file &&
--
1.8.3.3.1016.g4f0baba
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] t8001/t8002 (blame): modernize style
In particular,
- indent with tabs
- cuddle test description and opening body quote with test_expect_foo
- normalize test descriptions and case
- remove whitepsace following redirection operator
- use standardized filenames (such as "actual", "expected")
Signed-off-by: Eric Sunshine
---
t/annotate-tests.sh | 221 +++-
t/t8001-annotate.sh | 6 +-
t/t8002-blame.sh| 12 ++-
3 files changed, 127 insertions(+), 112 deletions(-)
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index c56a77d..3aa6964 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -2,11 +2,11 @@
# sourced from t8001-annotate.sh and t8002-blame.sh.
check_count () {
- head=
- case "$1" in -h) head="$2"; shift; shift ;; esac
- echo "$PROG file $head" >&4
- $PROG file $head >.result || return 1
- cat .result | perl -e '
+ head= &&
+ case "$1" in -h) head="$2"; shift; shift ;; esac &&
+ echo "$PROG file $head" >&4 &&
+ $PROG file $head >actual &&
+ perl -e '
my %expect = (@ARGV);
my %count = map { $_ => 0 } keys %expect;
while () {
@@ -31,107 +31,114 @@ check_count () {
print STDERR "Author $author (expected $value,
attributed $count) $ok\n";
}
exit($bad);
- ' "$@"
+ ' "$@" file &&
- echo "lazy dog" >>file &&
- git add file &&
- GIT_AUTHOR_NAME="A" GIT_AUTHOR_EMAIL="a...@test.git" git commit -a -m
"Initial."'
-
-test_expect_success \
-'check all lines blamed on A' \
-'check_count A 2'
-
-test_expect_success \
-'Setup new lines blamed on B' \
-'echo "2A quick brown fox jumps over the" >>file &&
- echo "lazy dog" >> file &&
- GIT_AUTHOR_NAME="B" GIT_AUTHOR_EMAIL="b...@test.git" git commit -a -m
"Second."'
-
-test_expect_success \
-'Two lines blamed on A, two on B' \
-'check_count A 2 B 2'
-
-test_expect_success \
-'merge-setup part 1' \
-'git checkout -b branch1 master &&
- echo "3A slow green fox jumps into the" >> file &&
- echo "well." >> file &&
- GIT_AUTHOR_NAME="B1" GIT_AUTHOR_EMAIL="b...@test.git" git commit -a -m
"Branch1-1"'
-
-test_expect_success \
-'Two lines blamed on A, two on B, two on B1' \
-'check_count A 2 B 2 B1 2'
-
-test_expect_success \
-'merge-setup part 2' \
-'git checkout -b branch2 master &&
- sed -e "s/2A quick brown/4A quick brown lazy dog/" < file > file.new &&
- mv file.new file &&
- GIT_AUTHOR_NAME="B2" GIT_AUTHOR_EMAIL="b...@test.git" git commit -a -m
"Branch2-1"'
-
-test_expect_success \
-'Two lines blamed on A, one on B, one on B2' \
-'check_count A 2 B 1 B2 1'
-
-test_expect_success \
-'merge-setup part 3' \
-'git pull . branch1'
-
-test_expect_success \
-'Two lines blamed on A, one on B, two on B1, one on B2' \
-'check_count A 2 B 1 B1 2 B2 1'
-
-test_expect_success \
-'Annotating an old revision works' \
-'check_count -h master A 2 B 2'
-
-test_expect_success \
-'Annotating an old revision works' \
-'check_count -h master^ A 2'
-
-test_expect_success \
-'merge-setup part 4' \
-'echo "evil merge." >>file &&
- git commit -a --amend'
-
-test_expect_success \
-'Two lines blamed on A, one on B, two on B1, one on B2, one on A U Thor' \
-'check_count A 2 B 1 B1 2 B2 1 "A U Thor" 1'
-
-test_expect_success \
-'an incomplete line added' \
-'echo "incomplete" | tr -d "\\012" >>file &&
-GIT_AUTHOR_NAME="C" GIT_AUTHOR_EMAIL="c...@test.git" git commit -a -m
"Incomplete"'
-
-test_expect_success \
-'With incomplete lines.' \
-'check_count A 2 B 1 B1 2 B2 1 "A U Thor" 1 C 1'
-
-test_expect_success \
-'some edit' \
-'mv file file.orig &&
-{
- cat file.orig &&
- echo
-} | sed -e "s/^3A/99/" -e "/^1A/d" -e "/^incomplete/d" > file &&
-echo "incomplete" | tr -d "\\012" >>file &&
-GIT_AUTHOR_NAME="D" GIT_AUTHOR_EMAIL="d...@test.git" git commit -a -m
"edit"'
-
-test_expect_success \
-'some edit' \
-'check_count A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1'
-
-test_expect_success \
-'an obfuscated email added' \
-'echo "No robots allowed" > file.new &&
- cat file >> file.new &&
- mv file.new file &&
- GIT_AUTHOR_NAME="E" GIT_AUTHOR_EMAIL="E at test dot git" git commit -a -m
"norobots"'
-
-test_expect_success \
-'obfuscated email parsed' \
-'check_count A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1 E 1'
+test_expect_success 'setup A lines' '
+ echo "1A quick brown fox jumps over the" >file &&
+ echo "lazy dog" >>file &&
+ git add file &&
+ GIT_AUTHOR_NAME="A" GIT_AUTHOR_EMAIL="a...@test.git" \
+ git commit -a -m "Initial."
+'
+
+test_expect_success 'blame 1 author' '
+ check_count A 2
+'
+
+test_expect_success 'setup B lines' '
+ echo "2A quick brown fox jumps over the" >>file &&
+ echo "lazy dog" >>file &&
[PATCH 5/6] blame-options.txt: place each -L option variation on its own line
Standard practice in Git documentation is for each variation of an option (such as: -p / --porcelain) to be placed on its own line in the OPTIONS table. The -L option does not follow suit. It cuddles "-L ," and "-L :", separated by a comma. This is inconsistent and potentially confusing since the comma separating them is typeset the same as the comma in ",". Fix this by placing each variation on its own line. Signed-off-by: Eric Sunshine --- Documentation/blame-options.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt index e9f984b..624b353 100644 --- a/Documentation/blame-options.txt +++ b/Documentation/blame-options.txt @@ -9,7 +9,8 @@ --show-stats:: Include additional statistics at the end of blame output. --L ,, -L ::: +-L ,:: +-L ::: Annotate only the given line range. and can take one of these forms: -- 1.8.3.3.1016.g4f0baba -- 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 3/6] t8001/t8002 (blame): add blame -L tests
With the exception of a couple "corner case" checks in t8003 (and some
indirect tests in t4211 of -L parsing code shared by log -L), there is
no systematic checking of blame -L. Add tests to check blame -L
directly.
Signed-off-by: Eric Sunshine
---
t/annotate-tests.sh | 104 ++--
1 file changed, 101 insertions(+), 3 deletions(-)
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 3aa6964..b6a7478 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -3,9 +3,17 @@
check_count () {
head= &&
- case "$1" in -h) head="$2"; shift; shift ;; esac &&
- echo "$PROG file $head" >&4 &&
- $PROG file $head >actual &&
+ options= &&
+ while :
+ do
+ case "$1" in
+ -h) head="$2"; shift; shift ;;
+ -*) options="$options $1"; shift ;;
+ *) break ;;
+ esac
+ done &&
+ echo "$PROG $options file $head" >&4 &&
+ $PROG $options file $head >actual &&
perl -e '
my %expect = (@ARGV);
my %count = map { $_ => 0 } keys %expect;
@@ -142,3 +150,93 @@ test_expect_success 'setup obfuscated email' '
test_expect_success 'blame obfuscated email' '
check_count A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1 E 1
'
+
+test_expect_success 'blame -L 1 (all)' '
+ check_count -L1 A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1 E 1
+'
+
+test_expect_success 'blame -L , (all)' '
+ check_count -L, A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1 E 1
+'
+
+test_expect_success 'blame -L X (X to end)' '
+ check_count -L5 B1 1 C 1 D 1 "A U Thor" 1
+'
+
+test_expect_success 'blame -L X, (X to end)' '
+ check_count -L5, B1 1 C 1 D 1 "A U Thor" 1
+'
+
+test_expect_success 'blame -L ,Y (up to Y)' '
+ check_count -L,3 A 1 B2 1 E 1
+'
+
+test_expect_success 'blame -L X,X' '
+ check_count -L3,3 B2 1
+'
+
+test_expect_success 'blame -L X,Y' '
+ check_count -L3,6 B 1 B1 1 B2 1 D 1
+'
+
+test_expect_success 'blame -L Y,X (undocumented)' '
+ check_count -L6,3 B 1 B1 1 B2 1 D 1
+'
+
+test_expect_success 'blame -L X,+1' '
+ check_count -L3,+1 B2 1
+'
+
+test_expect_success 'blame -L X,+N' '
+ check_count -L3,+4 B 1 B1 1 B2 1 D 1
+'
+
+test_expect_success 'blame -L X,-1' '
+ check_count -L3,-1 B2 1
+'
+
+test_expect_success 'blame -L X,-N' '
+ check_count -L6,-4 B 1 B1 1 B2 1 D 1
+'
+
+test_expect_success 'blame -L /RE/ (RE to end)' '
+ check_count -L/evil/ C 1 "A U Thor" 1
+'
+
+test_expect_success 'blame -L /RE/,/RE2/' '
+ check_count -L/robot/,/green/ A 1 B 1 B2 1 D 1 E 1
+'
+
+test_expect_success 'blame -L X,/RE/' '
+ check_count -L5,/evil/ B1 1 D 1 "A U Thor" 1
+'
+
+test_expect_success 'blame -L /RE/,Y' '
+ check_count -L/99/,7 B1 1 D 1 "A U Thor" 1
+'
+
+test_expect_success 'blame -L /RE/,+N' '
+ check_count -L/99/,+3 B1 1 D 1 "A U Thor" 1
+'
+
+test_expect_success 'blame -L /RE/,-N' '
+ check_count -L/99/,-3 B 1 B2 1 D 1
+'
+
+test_expect_success 'blame -L X (X > nlines)' '
+ test_must_fail $PROG -L12345 file
+'
+
+test_expect_success 'blame -L ,Y (Y > nlines)' '
+ test_must_fail $PROG -L,12345 file
+'
+
+test_expect_success 'blame -L bogus' '
+ test_must_fail $PROG -L file &&
+ test_must_fail $PROG -L1,+ file &&
+ test_must_fail $PROG -L1,- file &&
+ test_must_fail $PROG -LX file &&
+ test_must_fail $PROG -L1,X file &&
+ test_must_fail $PROG -L1,+N file &&
+ test_must_fail $PROG -L1,-N file
+'
--
1.8.3.3.1016.g4f0baba
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] blame-options.txt: explain that -L and are optional
The ability to omit either end of the -L range is a handy but undocumented shortcut, and is thus not easily discovered. Fix this shortcoming. Signed-off-by: Eric Sunshine --- Documentation/blame-options.txt | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt index 624b353..4e55b15 100644 --- a/Documentation/blame-options.txt +++ b/Documentation/blame-options.txt @@ -11,8 +11,11 @@ -L ,:: -L ::: - Annotate only the given line range. and can take - one of these forms: + Annotate only the given line range. and are optional. + ``-L '' or ``-L ,'' spans from to end of file. + ``-L ,'' spans from start of file to . ++ + and can take one of these forms: include::line-range-format.txt[] -- 1.8.3.3.1016.g4f0baba -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] line-range: fix "blame -L X,-N" regression
"blame -L X,-N" is documented as blaming "N lines ending at X". In
practice, the behavior is achieved by swapping the two range endpoints
if the second is less than the first. 25ed3412 (Refactor parse_loc;
2013-03-28) broke this interpretation by removing the swapping code from
blame.c and failing to add it to line-range.c along with other code
relocated from blame.c. Thus, such a range is effectively treated as
empty. Fix this regression.
Signed-off-by: Eric Sunshine
---
line-range.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/line-range.c b/line-range.c
index 8faf943..3942475 100644
--- a/line-range.c
+++ b/line-range.c
@@ -211,6 +211,8 @@ int parse_range_arg(const char *arg, nth_line_fn_t
nth_line_cb,
void *cb_data, long lines, long *begin, long *end,
const char *path)
{
+ *begin = *end = 0;
+
if (*arg == ':') {
arg = parse_range_funcname(arg, nth_line_cb, cb_data, lines,
begin, end, path);
if (!arg || *arg)
@@ -226,6 +228,11 @@ int parse_range_arg(const char *arg, nth_line_fn_t
nth_line_cb,
if (*arg)
return -1;
+ if (*begin && *end && *end < *begin) {
+ long tmp;
+ tmp = *end; *end = *begin; *begin = tmp;
+ }
+
return 0;
}
--
1.8.3.3.1016.g4f0baba
--
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] Fix some sparse warnings
On 07/16/2013 10:53 PM, Philip Oakley wrote:
>
> Does anyone run the "new static checker called 'Stack' that precisely
> identifies unstable code"? [though the paper's conclusion says 'All
> Stack source code will be publicly available.' which suggests it's not
> yet available]
>
So I started using the clang code analyzer on git. One of the
first warnings actually is this:
object.c:241:7: warning: Branch condition evaluates to a garbage value
if (!eaten)
So that part of object.c lookx like this:
struct object *parse_object(const unsigned char *sha1)
{
int eaten;
...
obj = parse_object_buffer(sha1, type, size, buffer, &eaten);
if (!eaten)
free(buffer);
}
And the parse_object_buffer looks like this with respect to the eaten
variable:
struct object *parse_object_buffer(...)
{
int eaten = 0;
if (something)
return NULL;
...
if (something_different)
eaten=1;
*eaten_p = eaten;
}
So what might happen is, that parse_object_buffer exits early, without
executing
*eaten_p = eaten;
Then in the parse_object function eaten was never initialized nor set
inside the call to parse_object_buffer. Then it is obvious that the
free(buffer) is executed depending on garbage left on the stack.
Definitely something what we want to change.
The obvious way to repair this would be to just initialize the eaten variable
inside parse_object.
struct object *parse_object(const unsigned char *sha1)
{
int eaten=0;
...
However I'd like to propose another solution:
In parse_object_buffer we do not have a local eaten variable, but
directly write to *eaten_p. That would be the following patch.
Was there a particular idea or goal behind first having a local eaten
variable, which later near the correct return of the function was used to set
the
eaten_p?
Thanks,
Stefan
signature.asc
Description: OpenPGP digital signature
[PATCH] parse_object_buffer: Correct freeing the buffer.
If we exit early in the function parse_object_buffer, we did not
write to *eaten_p. Then the calling function parse_object, which looks
like the following with respect to the eaten variable, cannot rely on a
proper value set in eaten, hence the freeing of the buffer depends
on random values in memory.
struct object *parse_object(const unsigned char *sha1)
{
int eaten;
...
obj = parse_object_buffer(sha1, type, size, buffer, &eaten);
if (!eaten)
free(buffer);
}
This change makes sure, the buffer freeing condition is deterministic.
Signed-off-by: Stefan Beller
---
object.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/object.c b/object.c
index cbc7333..d8a4b1f 100644
--- a/object.c
+++ b/object.c
@@ -145,7 +145,7 @@ struct object *lookup_unknown_object(const unsigned char
*sha1)
struct object *parse_object_buffer(const unsigned char *sha1, enum object_type
type, unsigned long size, void *buffer, int *eaten_p)
{
struct object *obj;
- int eaten = 0;
+ *eaten_p = 0;
obj = NULL;
if (type == OBJ_BLOB) {
@@ -164,7 +164,7 @@ struct object *parse_object_buffer(const unsigned char
*sha1, enum object_type t
if (!tree->object.parsed) {
if (parse_tree_buffer(tree, buffer, size))
return NULL;
- eaten = 1;
+ *eaten_p = 1;
}
}
} else if (type == OBJ_COMMIT) {
@@ -174,7 +174,7 @@ struct object *parse_object_buffer(const unsigned char
*sha1, enum object_type t
return NULL;
if (!commit->buffer) {
commit->buffer = buffer;
- eaten = 1;
+ *eaten_p = 1;
}
obj = &commit->object;
}
@@ -191,7 +191,6 @@ struct object *parse_object_buffer(const unsigned char
*sha1, enum object_type t
}
if (obj && obj->type == OBJ_NONE)
obj->type = type;
- *eaten_p = eaten;
return obj;
}
--
1.8.3.3.754.g9c3c367.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Git-P4 Bug With Filename Case Change
Hello Git Developers, We recently have moved our project from Git to Perforce and those of us who prefer Git still are using Git p4 to stay in Git land. One of the files in our repository was renamed while still in Git, but the rename only consisted of a case change of a character in the name. Now, on an OS X box with a case insensitive file system (not sure if that matters), one of our guys cloned from perforce with Git p4 and used @all to get all history. When this operation is finished, the file name is in its original state, not the newer renamed state. Perforce doesn't respect that file as being in the repository. We noticed this after making a local Git commit and upon issuing a Git p4 submit, things go haywire with "file(s) not opened on this client" and nothing getting submitted. Aaron Dwyer Senior Software Architect Imagination Technologies -- 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] Fix some sparse warnings
On 07/18/2013 12:08 AM, Stefan Beller wrote: > > So I started using the clang code analyzer on git. One of the > first warnings actually is this: > So in case somebody else would also like to play around with the clang static code analyzer: # get clang: cd git clone http://llvm.org/git/llvm.git cd llvm/tools git clone http://llvm.org/git/clang cd mkdir build && cd build cmake .. # if desired make install # in the Makefile of git change CC = /llvm/tools/clang/tools/scan-build/ccc-analyzer # then obviously: make clean make signature.asc Description: OpenPGP digital signature
Re: [PATCH] Fix some sparse warnings
Stefan Beller writes:
> And the parse_object_buffer looks like this with respect to the eaten
> variable:
> struct object *parse_object_buffer(...)
> {
> int eaten = 0;
> if (something)
> return NULL;
> ...
> if (something_different)
> eaten=1;
> *eaten_p = eaten;
> }
> ...
> Was there a particular idea or goal behind first having a local eaten
> variable, which later near the correct return of the function was used to set
> the
> eaten_p?
I didn't run "blame" to see the evolution of this function, but I
suspect that the original code, when the "eaten" local variable was
introduced, very much tried to do exactly what you suspect. The
early return codepaths you see in today's code may be much newer,
added without much thinking about the exact issue you are bringing
up.
--
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/19] Index-v5
Thomas Gummerer writes: > Ah ok, I understand. I think it's best to add a GIT_INDEX_VERSION=x > config option to config.mak, where x is the index version that should be > tested. Whatever you do, please do not call it GIT_INDEX_VERSION _if_ it is only to be used while testing. Have string "TEST" somewhere in the name, and make t/test-lib-functions.sh take notice. Currently, the way for the user to show the preference as to which index version to use is to explicitly set the version once, and then we will (try to) propagate it inside the repository. I would not mind if we add a mechanism to make write_index() notice the environment variable GIT_INDEX_VERSION and write the index in that version. But that is conceptually very different from whatever you give "make VARIABLE=blah" from the command line when building Git (or set in config.mak which amounts to the same thing). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] diff: pass the whole diff_options to diffcore_apply_filter()
The --diff-filter= option given by the user is kept as a
string, and passed to the underlying diffcore_apply_filter()
function as a string for each resulting path we run number of
strchr() to see if each class of change among ACDMRTXUB is meant to
be given.
Change the function signature to pass the whole diff_options, so
that we can pre-parse this string in the next patch.
Signed-off-by: Junio C Hamano
---
diff.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index 649ec86..41c64f2 100644
--- a/diff.c
+++ b/diff.c
@@ -4509,11 +4509,13 @@ free_queue:
}
}
-static void diffcore_apply_filter(const char *filter)
+static void diffcore_apply_filter(struct diff_options *options)
{
int i;
struct diff_queue_struct *q = &diff_queued_diff;
struct diff_queue_struct outq;
+ const char *filter = options->filter;
+
DIFF_QUEUE_CLEAR(&outq);
if (!filter)
@@ -4661,7 +4663,7 @@ void diffcore_std(struct diff_options *options)
if (!options->found_follow)
/* See try_to_follow_renames() in tree-diff.c */
diff_resolve_rename_copy();
- diffcore_apply_filter(options->filter);
+ diffcore_apply_filter(options);
if (diff_queued_diff.nr && !DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
DIFF_OPT_SET(options, HAS_CHANGES);
--
1.8.3.3-962-gf04df43
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] Deprecating "diff-files -q"
The "-q" option given to "git diff-files" is a remnant of the
"show-diff" command, the precursor of today's "git diff-files" (back
then, we didn't even have "git" potty. The user literally typed
"show-diff", not "git show-diff").
ca2a0798 ([PATCH] Add "-q" option to show-diff.c, 2005-04-15) added
that option. Back then, we did not have pathspec matching, and we
iterated over command line arguments, and required all of them exist
as filesystem entities. "-q" was a way to defeat that "you name a
file, it must exist in the working tree" safety, and also at the
same time not give output for such a file that was removed from the
working tree.
These days, the command line parsing infrastructure has changed
vastly since "show-diff" days, and the former "safety" is enforced
by the generalized revision parser ("is it a path or is it a rev?")
code and the "--" delimiter on the command line is the way to defeat
it. The latter is done by giving a filtering specification that
lack D to the "--diff-filter", e.g. "--diff-filter=ACMRTUB".
This is however a bit cumbersome to type. This miniseries updates
the diff-filter mechanism to let you say --diff-filter=d (lowercase)
to express that you are interested in the changes in general, but
not the changes in the 'D' class (i.e. deletion).
The last step tweaks the command line parser of "git diff-files"
(and "git diff" without any object on the command line, which goes
to the same codepath) and "git diff --no-index" to notice "-q", warn
and then turn it into "--diff-filter=d". We should remove the
entire thing at a major version bump, like Git 2.0.
This is still a bit rough, without any documentation updates nor
tests.
Junio C Hamano (6):
diff: pass the whole diff_options to diffcore_apply_filter()
diff: factor out match_filter()
diff: preparse --diff-filter string argument
diff: reject unknown change class given to --diff-filter
diff: allow lowercase letter to specify what change class to exclude
diff: deprecate -q option to diff-files
diff-lib.c | 8 ++--
diff-no-index.c | 7 +++-
diff.c | 125 ++--
diff.h | 7 +++-
4 files changed, 118 insertions(+), 29 deletions(-)
--
1.8.3.3-962-gf04df43
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] diff: reject unknown change class given to --diff-filter
We used to accept "git diff --diff-filter=Q" (note that there is no such change class 'Q') silently and showed no output (because there is no such change class 'Q'). Error out when such an input is given. Signed-off-by: Junio C Hamano --- diff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diff.c b/diff.c index 03f10e6..3d37b56 100644 --- a/diff.c +++ b/diff.c @@ -3537,7 +3537,7 @@ static int parse_diff_filter_opt(const char *optarg, struct diff_options *opt) bit = (0 <= optch && optch <= 'Z') ? filter_bit[optch] : 0; if (!bit) - continue; /* ignore unknown ones, like we always have */ + return optarg[i]; opt->filter |= bit; } return 0; -- 1.8.3.3-962-gf04df43 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] diff: deprecate -q option to diff-files
This reimplements the ancient "-q" option to "git diff-files" that
was inherited from "show-diff -q" in terms of "--diff-filter=d", and
issue a warning against the use of the former.
Incidentally this also tentatively fix "git diff --no-index" to
honor "-q" and hide deletions; the use will get the same warning.
We should remove the support for "-q" in Git 2.0.
Signed-off-by: Junio C Hamano
---
diff-lib.c | 8 +++-
diff-no-index.c | 7 +--
diff.c | 8
diff.h | 2 ++
4 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index f35de0f..4634b29 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -86,10 +86,12 @@ int run_diff_files(struct rev_info *revs, unsigned int
option)
{
int entries, i;
int diff_unmerged_stage = revs->max_count;
- int silent_on_removed = option & DIFF_SILENT_ON_REMOVED;
unsigned ce_option = ((option & DIFF_RACY_IS_MODIFIED)
? CE_MATCH_RACY_IS_DIRTY : 0);
+ if (option & DIFF_SILENT_ON_REMOVED)
+ handle_deprecated_show_diff_q(&revs->diffopt);
+
diff_set_mnemonic_prefix(&revs->diffopt, "i/", "w/");
if (diff_unmerged_stage < 0)
@@ -136,8 +138,6 @@ int run_diff_files(struct rev_info *revs, unsigned int
option)
perror(ce->name);
continue;
}
- if (silent_on_removed)
- continue;
wt_mode = 0;
}
dpath->mode = wt_mode;
@@ -203,8 +203,6 @@ int run_diff_files(struct rev_info *revs, unsigned int
option)
perror(ce->name);
continue;
}
- if (silent_on_removed)
- continue;
diff_addremove(&revs->diffopt, '-', ce->ce_mode,
ce->sha1, !is_null_sha1(ce->sha1),
ce->name, 0);
diff --git a/diff-no-index.c b/diff-no-index.c
index 74da659..a788a5f 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -187,7 +187,7 @@ void diff_no_index(struct rev_info *revs,
{
int i, prefixlen;
int no_index = 0;
- unsigned options = 0;
+ unsigned deprecated_show_diff_q_option_used = 0;
const char *paths[2];
/* Were we asked to do --no-index explicitly? */
@@ -225,7 +225,7 @@ void diff_no_index(struct rev_info *revs,
if (!strcmp(argv[i], "--no-index"))
i++;
else if (!strcmp(argv[i], "-q")) {
- options |= DIFF_SILENT_ON_REMOVED;
+ deprecated_show_diff_q_option_used = 1;
i++;
}
else if (!strcmp(argv[i], "--"))
@@ -260,6 +260,9 @@ void diff_no_index(struct rev_info *revs,
revs->max_count = -2;
diff_setup_done(&revs->diffopt);
+ if (deprecated_show_diff_q_option_used)
+ handle_deprecated_show_diff_q(&revs->diffopt);
+
setup_diff_pager(&revs->diffopt);
DIFF_OPT_SET(&revs->diffopt, EXIT_WITH_STATUS);
diff --git a/diff.c b/diff.c
index 2d0b5e3..78819ba 100644
--- a/diff.c
+++ b/diff.c
@@ -3570,6 +3570,14 @@ static int parse_diff_filter_opt(const char *optarg,
struct diff_options *opt)
return 0;
}
+/* Used only by "diff-files" and "diff --no-index" */
+void handle_deprecated_show_diff_q(struct diff_options *opt)
+{
+ warning("'diff -q' and 'diff-files -q' are deprecated.");
+ warning("Use 'diff --diff-filter=d' instead to ignore deleted
filepairs.");
+ parse_diff_filter_opt("d", opt);
+}
+
int diff_opt_parse(struct diff_options *options, const char **av, int ac)
{
const char *arg = av[0];
diff --git a/diff.h b/diff.h
index a367207..5237d63 100644
--- a/diff.h
+++ b/diff.h
@@ -341,6 +341,8 @@ extern int parse_rename_score(const char **cp_p);
extern long parse_algorithm_value(const char *value);
+extern void handle_deprecated_show_diff_q(struct diff_options *);
+
extern int print_stat_summary(FILE *fp, int files,
int insertions, int deletions);
extern void setup_diff_pager(struct diff_options *);
--
1.8.3.3-962-gf04df43
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] diff: allow lowercase letter to specify what change class to exclude
In order to express "we do not care about deletions", we had to say
"--diff-filter=ACMRTXUB", giving all the possible change class
except for the one we do not want, "D".
This is cumbersome. As all the change classes are in uppercase,
allow their lowercase counterpart to selectively exclude the class
from the output. When such a negated change class is in the input,
start the filter option with the full bits set.
This would allow us to express the old "show-diff -q" with
"git diff-files --diff-filter=d".
Signed-off-by: Junio C Hamano
---
diff.c | 29 -
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index 3d37b56..2d0b5e3 100644
--- a/diff.c
+++ b/diff.c
@@ -3532,13 +3532,40 @@ static int parse_diff_filter_opt(const char *optarg,
struct diff_options *opt)
int i, optch;
prepare_filter_bits();
+
+ /*
+* If there is a negation e.g. 'd' in the input, and we haven't
+* initialized the filter field with another --diff-filter, start
+* from full set of bits, except for AON.
+*/
+ if (!opt->filter) {
+ for (i = 0; (optch = optarg[i]) != '\0'; i++) {
+ if (optch < 'a' || 'z' < optch)
+ continue;
+ opt->filter = (1 << (ARRAY_SIZE(diff_status_letters) -
1)) - 1;
+ opt->filter &= ~filter_bit[DIFF_STATUS_FILTER_AON];
+ break;
+ }
+ }
+
for (i = 0; (optch = optarg[i]) != '\0'; i++) {
unsigned int bit;
+ int negate;
+
+ if ('a' <= optch && optch <= 'z') {
+ negate = 1;
+ optch = toupper(optch);
+ } else {
+ negate = 0;
+ }
bit = (0 <= optch && optch <= 'Z') ? filter_bit[optch] : 0;
if (!bit)
return optarg[i];
- opt->filter |= bit;
+ if (negate)
+ opt->filter &= ~bit;
+ else
+ opt->filter |= bit;
}
return 0;
}
--
1.8.3.3-962-gf04df43
--
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 3/6] diff: preparse --diff-filter string argument
Instead of running strchr() on the list of status characters over
and over again, parse the --diff-filter option into bitfields and
use the bits to see if the change to the filepair matches the status
requested.
Signed-off-by: Junio C Hamano
---
diff.c | 63 ---
diff.h | 5 -
2 files changed, 60 insertions(+), 8 deletions(-)
diff --git a/diff.c b/diff.c
index 0220c19..03f10e6 100644
--- a/diff.c
+++ b/diff.c
@@ -3496,6 +3496,53 @@ static int parse_submodule_opt(struct diff_options
*options, const char *value)
return 1;
}
+static const char diff_status_letters[] = {
+ DIFF_STATUS_ADDED,
+ DIFF_STATUS_COPIED,
+ DIFF_STATUS_DELETED,
+ DIFF_STATUS_MODIFIED,
+ DIFF_STATUS_RENAMED,
+ DIFF_STATUS_TYPE_CHANGED,
+ DIFF_STATUS_UNKNOWN,
+ DIFF_STATUS_UNMERGED,
+ DIFF_STATUS_FILTER_AON,
+ DIFF_STATUS_FILTER_BROKEN,
+ '\0',
+};
+
+static unsigned int filter_bit['Z' + 1];
+
+static void prepare_filter_bits(void)
+{
+ int i;
+
+ if (!filter_bit[DIFF_STATUS_ADDED]) {
+ for (i = 0; diff_status_letters[i]; i++)
+ filter_bit[(int) diff_status_letters[i]] = (1 << i);
+ }
+}
+
+static unsigned filter_bit_tst(char status, const struct diff_options *opt)
+{
+ return opt->filter & filter_bit[(int) status];
+}
+
+static int parse_diff_filter_opt(const char *optarg, struct diff_options *opt)
+{
+ int i, optch;
+
+ prepare_filter_bits();
+ for (i = 0; (optch = optarg[i]) != '\0'; i++) {
+ unsigned int bit;
+
+ bit = (0 <= optch && optch <= 'Z') ? filter_bit[optch] : 0;
+ if (!bit)
+ continue; /* ignore unknown ones, like we always have */
+ opt->filter |= bit;
+ }
+ return 0;
+}
+
int diff_opt_parse(struct diff_options *options, const char **av, int ac)
{
const char *arg = av[0];
@@ -3717,7 +3764,10 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
return argcount;
}
else if ((argcount = parse_long_opt("diff-filter", av, &optarg))) {
- options->filter = optarg;
+ int offending = parse_diff_filter_opt(optarg, options);
+ if (offending)
+ die("unknown change class '%c' in --diff-filter=%s",
+ offending, optarg);
return argcount;
}
else if (!strcmp(arg, "--abbrev"))
@@ -4513,11 +4563,11 @@ static int match_filter(const struct diff_options
*options, const struct diff_fi
{
return (((p->status == DIFF_STATUS_MODIFIED) &&
((p->score &&
- strchr(options->filter, DIFF_STATUS_FILTER_BROKEN)) ||
+ filter_bit_tst(DIFF_STATUS_FILTER_BROKEN, options)) ||
(!p->score &&
- strchr(options->filter, DIFF_STATUS_MODIFIED ||
+ filter_bit_tst(DIFF_STATUS_MODIFIED, options ||
((p->status != DIFF_STATUS_MODIFIED) &&
-strchr(options->filter, p->status)));
+filter_bit_tst(p->status, options)));
}
static void diffcore_apply_filter(struct diff_options *options)
@@ -4525,14 +4575,13 @@ static void diffcore_apply_filter(struct diff_options
*options)
int i;
struct diff_queue_struct *q = &diff_queued_diff;
struct diff_queue_struct outq;
- const char *filter = options->filter;
DIFF_QUEUE_CLEAR(&outq);
- if (!filter)
+ if (!options->filter)
return;
- if (strchr(filter, DIFF_STATUS_FILTER_AON)) {
+ if (filter_bit_tst(DIFF_STATUS_FILTER_AON, options)) {
int found;
for (i = found = 0; !found && i < q->nr; i++) {
if (match_filter(options, q->queue[i]))
diff --git a/diff.h b/diff.h
index 78b4091..a367207 100644
--- a/diff.h
+++ b/diff.h
@@ -103,12 +103,15 @@ enum diff_words_type {
};
struct diff_options {
- const char *filter;
const char *orderfile;
const char *pickaxe;
const char *single_follow;
const char *a_prefix, *b_prefix;
unsigned flags;
+
+ /* diff-filter bits */
+ unsigned int filter;
+
int use_color;
int context;
int interhunkcontext;
--
1.8.3.3-962-gf04df43
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] diff: factor out match_filter()
diffcore_apply_filter() checks if a filepair matches the filter
given with the "--diff-filter" option for each input filepairs with
a fairly complex expression in two places.
Create a helper function and call it.
Signed-off-by: Junio C Hamano
---
diff.c | 29 +
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/diff.c b/diff.c
index 41c64f2..0220c19 100644
--- a/diff.c
+++ b/diff.c
@@ -4509,6 +4509,17 @@ free_queue:
}
}
+static int match_filter(const struct diff_options *options, const struct
diff_filepair *p)
+{
+ return (((p->status == DIFF_STATUS_MODIFIED) &&
+((p->score &&
+ strchr(options->filter, DIFF_STATUS_FILTER_BROKEN)) ||
+ (!p->score &&
+ strchr(options->filter, DIFF_STATUS_MODIFIED ||
+ ((p->status != DIFF_STATUS_MODIFIED) &&
+strchr(options->filter, p->status)));
+}
+
static void diffcore_apply_filter(struct diff_options *options)
{
int i;
@@ -4524,14 +4535,7 @@ static void diffcore_apply_filter(struct diff_options
*options)
if (strchr(filter, DIFF_STATUS_FILTER_AON)) {
int found;
for (i = found = 0; !found && i < q->nr; i++) {
- struct diff_filepair *p = q->queue[i];
- if (((p->status == DIFF_STATUS_MODIFIED) &&
-((p->score &&
- strchr(filter, DIFF_STATUS_FILTER_BROKEN)) ||
- (!p->score &&
- strchr(filter, DIFF_STATUS_MODIFIED ||
- ((p->status != DIFF_STATUS_MODIFIED) &&
-strchr(filter, p->status)))
+ if (match_filter(options, q->queue[i]))
found++;
}
if (found)
@@ -4549,14 +4553,7 @@ static void diffcore_apply_filter(struct diff_options
*options)
/* Only the matching ones */
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
-
- if (((p->status == DIFF_STATUS_MODIFIED) &&
-((p->score &&
- strchr(filter, DIFF_STATUS_FILTER_BROKEN)) ||
- (!p->score &&
- strchr(filter, DIFF_STATUS_MODIFIED ||
- ((p->status != DIFF_STATUS_MODIFIED) &&
-strchr(filter, p->status)))
+ if (match_filter(options, p))
diff_q(&outq, p);
else
diff_free_filepair(p);
--
1.8.3.3-962-gf04df43
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/6] Make "git show -s" easier to discover for users
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: [PATCH] .mailmap: René Scharfe has a new email address
René Scharfe writes: > Signed-off-by: René Scharfe > --- > I failed to log on to the dyn.com website in time and lost my old free > DNS entry. :-/ Thanks. Now the real issue is how we verify this patch is from the real René whose longtime contribution we all appreciate ;-). > > .mailmap | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/.mailmap b/.mailmap > index 345cce6..5c16adf 100644 > --- a/.mailmap > +++ b/.mailmap > @@ -78,7 +78,7 @@ Petr Baudis > Philippe Bruhat > Ralf Thielow > Ramsay Allan Jones > -René Scharfe > +René Scharfe > Robert Fitzsimons > Robert Zeh > Sam Vilain -- 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] .mailmap: Combine more (email, name) to individual persons
Stefan Beller writes: > I got more responses from people regarding the .mailmap file. > All added persons gave permission to add them to the .mailmap file. 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: [PATCH] do_one_ref(): save and restore value of current_ref
Michael Haggerty writes: > If do_one_ref() is called recursively, then the inner call should not > permanently overwrite the value stored in current_ref by the outer > call. Aside from the tiny optimization loss, peel_ref() expects the > value of current_ref not to change across a call to peel_entry(). But > in the presence of replace references that assumption could be > violated by a recursive call to do_one_ref: > > do_for_each_entry() > do_one_ref() > builtin/describe.c:get_name() > peel_ref() > peel_entry() > peel_object () > deref_tag_noverify() > parse_object() > lookup_replace_object() > do_lookup_replace_object() > prepare_replace_object() > do_for_each_ref() > do_for_each_entry() > do_for_each_entry_in_dir() > do_one_ref() > > The inner call to do_one_ref() was unconditionally setting current_ref > to NULL when it was done, causing peel_ref() to perform an invalid > memory access. > > So change do_one_ref() to save the old value of current_ref before > overwriting it, and restore the old value afterward rather than > setting it to NULL. > > Reported by: Mantas Mikulėnas > > Signed-off-by: Michael Haggerty > --- Thanks. s/Reported by:/Reported-by:/ and lose the extra blank line after it? I wonder if we can have an easy reproduction recipe in our tests. -- 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] TIG: Implement mkstemps() work-around for platforms lacking it
Giving this one last kick to make absolutely sure that nobody disagrees
with allowing this code to be included into tig, which does not limit
to a specific version of the GPL (version 2 in the case of git, any
version equal to or newer than 2 in the case of tig), pursuant to
paragraph 9 of said license.
On 07/09/2013 11:33 AM, Drew Northup wrote:
The function mkstemps() isn't available in all libc implementations. In
glibc it first became available in 2.11, so platforms such as RHEL 5&
Slackware 13 lack it. This is likely true of many non-LINUX platforms
as well.
This fixes breakage that was introduced with a0fdac29 "Create temporary
file with name as suffix."
Signed-off-by: Drew Northup
---
This work-around is taken from Git and was inspired by code in libiberty.
It is presumed that this isn't a problem due to compatible license terms.
A (virtually identical) version of this available in
https://github.com/n1xim/tig/tree/mkstemps_wkarnd (differences only in
the commit message).
configure.ac | 4
io.c | 77
io.h | 14 +++
3 files changed, 95 insertions(+)
diff --git a/configure.ac b/configure.ac
index 8dd2508..40e1f85 100644
--- a/configure.ac
+++ b/configure.ac
@@ -21,6 +21,10 @@ AC_SUBST(CURSES_LIB)
AM_ICONV
+dnl Not all platforms have mkstemps
+AC_CHECK_FUNC([mkstemps], [AC_DEFINE([HAVE_MKSTEMPS], [1],
+ [Define if mkstemps is available.])])
+
AC_PROG_CC
AC_CHECK_PROGS(ASCIIDOC, [asciidoc], [false])
diff --git a/io.c b/io.c
index 3ff1d1c..f1b6fbc 100644
--- a/io.c
+++ b/io.c
@@ -237,6 +237,83 @@ encoding_convert(struct encoding *encoding, char *line)
}
/*
+ * Compatibility: no mkstemps()
+ */
+
+/* Adapted from libiberty's mkstemp.c via Git's wrapper.c. */
+
+#undef TMP_MAX
+#define TMP_MAX 16384
+
+int tig_mkstemps_mode(char *pattern, int suffix_len, int mode)
+{
+ static const char letters[] =
+ "abcdefghijklmnopqrstuvwxyz"
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+ "0123456789";
+ static const int num_letters = 62;
+ uint64_t value;
+ struct timeval tv;
+ char *template;
+ size_t len;
+ int fd, count;
+
+ len = strlen(pattern);
+
+ if (len< 6 + suffix_len) {
+ errno = EINVAL;
+ return -1;
+ }
+
+ if (strncmp(&pattern[len - 6 - suffix_len], "XX", 6)) {
+ errno = EINVAL;
+ return -1;
+ }
+
+ /*
+* Replace pattern's XX characters with randomness.
+* Try TMP_MAX different filenames.
+*/
+ gettimeofday(&tv, NULL);
+ value = ((size_t)(tv.tv_usec<< 16)) ^ tv.tv_sec ^ getpid();
+ template =&pattern[len - 6 - suffix_len];
+ for (count = 0; count< TMP_MAX; ++count) {
+ uint64_t v = value;
+ /* Fill in the random bits. */
+ template[0] = letters[v % num_letters]; v /= num_letters;
+ template[1] = letters[v % num_letters]; v /= num_letters;
+ template[2] = letters[v % num_letters]; v /= num_letters;
+ template[3] = letters[v % num_letters]; v /= num_letters;
+ template[4] = letters[v % num_letters]; v /= num_letters;
+ template[5] = letters[v % num_letters]; v /= num_letters;
+
+ fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode);
+ if (fd> 0)
+ return fd;
+ /*
+* Fatal error (EPERM, ENOSPC etc).
+* It doesn't make sense to loop.
+*/
+ if (errno != EEXIST)
+ break;
+ /*
+* This is a random value. It is only necessary that
+* the next TMP_MAX values generated by adding to
+* VALUE are different with (module 2^32).
+*/
+ value += ;
+ }
+ /* We return the null string if we can't find a unique file name. */
+ pattern[0] = '\0';
+ return -1;
+}
+
+int tigmkstemps(char *pattern, int suffix_len)
+{
+ return tig_mkstemps_mode(pattern, suffix_len, 0600);
+}
+
+/*
* Executing external commands.
*/
diff --git a/io.h b/io.h
index 646989d..8f43216 100644
--- a/io.h
+++ b/io.h
@@ -16,6 +16,9 @@
#include "tig.h"
+/* Needed for mkstemps workaround */
+#include
+
/*
* Argument array helpers.
*/
@@ -41,6 +44,17 @@ struct encoding *encoding_open(const char *fromcode);
char *encoding_convert(struct encoding *encoding, char *line);
/*
+ * Compatibility: no mkstemps()
+ */
+
+#ifndef HAVE_MKSTEMPS
+#define mkstemps tigmkstemps
+#endif
+
+int tigmkstemps(char *, int);
+int tig_mkstemps_mode(char *pattern, int suffix_len, int mode);
+
+/*
* Executing external commands.
*/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to m
[PATCH] TIG: Fix to reinstate proper operation with no arguments
Since c7d67ab running "tig" with no options has failed with the
error "tig: No revisions match the given arguments." This was due
to a change in how the arguments for the back-end git call was
being constructed. This change caused the blank field left in
place of "(encoding_arg)" when it is empty to not overwrite
"buf" which then caused the value in "buf" to be copied into
dst_argv twice. The resulting git command failed if there was no
available revision named "log" as shown in the trace.
>From the TIG_TRACE log:
git log log --no-color --pretty=raw --parents --parents --
fatal: bad revision 'log'
This fix works by teaching tig that when it is supplied with a
blank field in the source argument buffer that it should skip
over that field and continue instead of copying the previous
field value into the destination buffer a second time.
github issue # 167
Signed-off-by: Drew Northup
---
This should apply cleanly to the tig public master whether the
mkstemps() patch I wrote has been applied or not.
tig.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tig.c b/tig.c
index ba9ba98..1016cfe 100644
--- a/tig.c
+++ b/tig.c
@@ -3105,10 +3105,11 @@ static bool
format_append_arg(struct format_context *format, const char ***dst_argv, const
char *arg)
{
format->bufpos = 0;
+ int len = 0;
while (arg) {
char *next = strstr(arg, "%(");
- int len = next ? next - arg : strlen(arg);
+ len = next ? next - arg : strlen(arg);
if (len && !string_format_from(format->buf, &format->bufpos,
"%.*s", len, arg))
return FALSE;
@@ -3119,7 +3120,11 @@ format_append_arg(struct format_context *format, const
char ***dst_argv, const c
arg = next ? strchr(next, ')') + 1 : NULL;
}
- return argv_append(dst_argv, format->buf);
+ if(len){
+ return argv_append(dst_argv, format->buf);
+ } else {
+ return TRUE;
+ }
}
static bool
--
1.8.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
