Re: [RFC PATCH v5 2/3] grep: make PCRE2 aware of custom allocator
Hi Junio, On Thu, 3 Oct 2019, Junio C Hamano wrote: > Carlo Arenas writes: > > > On Tue, Aug 27, 2019 at 2:07 AM Johannes Schindelin > > wrote: > >> > >> Unfortunately, this is _still_ incorrect. > > ... > > Just to clarify, I think my patch accounts for that (haven't tested > > that assumption, but will do now that I have a windows box, probably > > even with mi-alloc) but yes, the only reason why there were references > > to NEDMALLOC was to isolate the code and make sure the fix was > > tackling the problem, it was not my intention to do so at the end, > > specially once we agreed that xmalloc should be used anyway. > > ... > > apologize for the delays, and will be fine using your squash, mine, > > the V6 RC (my preference) or dropping this series from pu if that > > would help clear the ugliness of pu for windows > > So,... have we seen any conclusion on this? Can any of you guys > give us a pointer to or copies of the candidate to be the final > solution of this topic, please? I still need https://github.com/git-for-windows/git/commit/719beb813e4f27f090696ce583df3e5f3c480545 and https://github.com/git-for-windows/git/commit/3369c322bbd95820b971701fef7db44b26dd826f to fix that part in Git for Windows' `shears/pu` branch (i.e. the continuously rebased patch thicket). Ciao, Dscho
Re: [RFC PATCH v5 2/3] grep: make PCRE2 aware of custom allocator
On Thu, Oct 3, 2019 at 1:09 AM Johannes Schindelin wrote: > I still need > https://github.com/git-for-windows/git/commit/719beb813e4f27f090696ce583df3e5f3c480545 > and > https://github.com/git-for-windows/git/commit/3369c322bbd95820b971701fef7db44b26dd826f > to fix that part in Git for Windows' `shears/pu` branch (i.e. the > continuously rebased patch thicket). or we could drop the current branch in pu and start again from scratch now that all of the required dependencies had been merged. apologize for the delays otherwise; $DAYJOB has taken a toll lately and even my new shiny windows dev box hasn't seen much action. will update here and in github shortly (which might mean up to this weekend, being realistic), but should be better code (since it is mostly Rene's) and better tested that way and hopefully won't cause more breakage (specially in Windows, sorry Dscho) Carlo
Re: [PATCH v2 2/8] fast-import: fix handling of deleted tags
Am 30.09.19 um 23:10 schrieb Elijah Newren:
> If our input stream includes a tag which is later deleted, we were not
> properly deleting it. We did have a step which would delete it, but we
> left a tag in the tag list noting that it needed to be updated, and the
> updating of annotated tags occurred AFTER ref deletion. So, when we
> record that a tag needs to be deleted, also remove it from the list of
> annotated tags to update.
>
> While this has likely been something that has not happened in practice,
> it will come up more in order to support nested tags. For nested tags,
> we either need to give temporary names to the intermediate tags and then
> delete them, or else we need to use the final name for the intermediate
> tags. If we use the final name for the intermediate tags, then in order
> to keep the sanity check that someone doesn't try to update the same tag
> twice, we need to delete the ref after creating the intermediate tag.
> So, either way nested tags imply the need to delete temporary inner tag
> references.
>
> Signed-off-by: Elijah Newren
> ---
> fast-import.c | 29 +
> t/t9300-fast-import.sh | 13 +
> 2 files changed, 42 insertions(+)
>
> diff --git a/fast-import.c b/fast-import.c
> index b44d6a467e..546da3a938 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -2793,6 +2793,35 @@ static void parse_reset_branch(const char *arg)
> b = new_branch(arg);
> read_next_command();
> parse_from(b);
> + if (b->delete && !strncmp(b->name, "refs/tags/", 10)) {
b->name is a NUL-terminated string; starts_with() could be used to avoid
the magic number 10.
> + /*
> + * Elsewhere, we call dump_branches() before dump_tags(),
> + * and dump_branches() will handle ref deletions first, so
> + * in order to make sure the deletion actually takes effect,
> + * we need to remove the tag from our list of tags to update.
> + *
> + * NEEDSWORK: replace list of tags with hashmap for faster
> + * deletion?
> + */
> + struct strbuf tag_name = STRBUF_INIT;
This adds a small memory leak.
> + struct tag *t, *prev = NULL;
> + for (t = first_tag; t; t = t->next_tag) {
> + strbuf_reset(&tag_name);
> + strbuf_addf(&tag_name, "refs/tags/%s", t->name);
> + if (!strcmp(b->name, tag_name.buf))
So the strbuf is used to prefix t->name with "refs/tags/", which we know
b->name starts with, and to compare the result with b->name. Removing
the "refs/tags/" prefix from b->name using skip_prefix() and comparing
the result with t->name would be easier.
> + break;
> + prev = t;
> + }
> + if (t) {
> + if (prev)
> + prev->next_tag = t->next_tag;
> + else
> + first_tag = t->next_tag;
> + if (!t->next_tag)
> + last_tag = prev;
> + /* There is no mem_pool_free(t) function to call. */
> + }
> + }
> if (command_buf.len > 0)
> unread_command_buf = 1;
> }
Here's a squashable patch for that:
---
fast-import.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 70cd3f0ff4..a109591406 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2779,6 +2779,7 @@ static void parse_new_tag(const char *arg)
static void parse_reset_branch(const char *arg)
{
struct branch *b;
+ const char *tag_name;
b = lookup_branch(arg);
if (b) {
@@ -2794,7 +2795,7 @@ static void parse_reset_branch(const char *arg)
b = new_branch(arg);
read_next_command();
parse_from(b);
- if (b->delete && !strncmp(b->name, "refs/tags/", 10)) {
+ if (b->delete && skip_prefix(b->name, "refs/tags/", &tag_name)) {
/*
* Elsewhere, we call dump_branches() before dump_tags(),
* and dump_branches() will handle ref deletions first, so
@@ -2804,12 +2805,9 @@ static void parse_reset_branch(const char *arg)
* NEEDSWORK: replace list of tags with hashmap for faster
* deletion?
*/
- struct strbuf tag_name = STRBUF_INIT;
struct tag *t, *prev = NULL;
for (t = first_tag; t; t = t->next_tag) {
- strbuf_reset(&tag_name);
- strbuf_addf(&tag_name, "refs/tags/%s", t->name);
- if (!strcmp(b->name, tag_name.buf))
+ if (!strcmp(t->name, tag_name))
break;
prev = t;
}
--
2.23.0
PATCH] remove duplicate #include directives
Found with "git grep '^#include ' '*.c' | sort | uniq -d".
Signed-off-by: René Scharfe
---
Patch formatted with --function-context for easier review.
builtin/am.c | 1 -
builtin/blame.c| 1 -
builtin/clone.c| 1 -
builtin/describe.c | 1 -
builtin/rev-list.c | 1 -
builtin/worktree.c | 1 -
object.c | 1 -
packfile.c | 1 -
shallow.c | 3 ---
unpack-trees.c | 1 -
10 files changed, 12 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index ee7305eaa6..b015e1d7d1 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1,40 +1,39 @@
/*
* Builtin "git am"
*
* Based on git-am.sh by Junio C Hamano.
*/
#define USE_THE_INDEX_COMPATIBILITY_MACROS
#include "cache.h"
#include "config.h"
#include "builtin.h"
#include "exec-cmd.h"
#include "parse-options.h"
#include "dir.h"
#include "run-command.h"
#include "quote.h"
#include "tempfile.h"
#include "lockfile.h"
#include "cache-tree.h"
#include "refs.h"
#include "commit.h"
#include "diff.h"
#include "diffcore.h"
#include "unpack-trees.h"
#include "branch.h"
#include "sequencer.h"
#include "revision.h"
#include "merge-recursive.h"
-#include "revision.h"
#include "log-tree.h"
#include "notes-utils.h"
#include "rerere.h"
#include "prompt.h"
#include "mailinfo.h"
#include "apply.h"
#include "string-list.h"
#include "packfile.h"
#include "repository.h"
/**
* Returns the length of the first line of msg.
*/
diff --git a/builtin/blame.c b/builtin/blame.c
index bfd537b344..9858d6b269 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1,32 +1,31 @@
/*
* Blame
*
* Copyright (c) 2006, 2014 by its authors
* See COPYING for licensing conditions
*/
#include "cache.h"
#include "config.h"
#include "color.h"
#include "builtin.h"
#include "repository.h"
#include "commit.h"
#include "diff.h"
#include "revision.h"
#include "quote.h"
#include "string-list.h"
#include "mailmap.h"
#include "parse-options.h"
#include "prio-queue.h"
#include "utf8.h"
#include "userdiff.h"
#include "line-range.h"
#include "line-log.h"
#include "dir.h"
#include "progress.h"
#include "object-store.h"
#include "blame.h"
-#include "string-list.h"
#include "refs.h"
static char blame_usage[] = N_("git blame [] [] []
[--] ");
diff --git a/builtin/clone.c b/builtin/clone.c
index 2048b6760a..9d73102c42 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1,44 +1,43 @@
/*
* Builtin "git clone"
*
* Copyright (c) 2007 Kristian Høgsberg ,
* 2008 Daniel Barkalow
* Based on git-commit.sh by Junio C Hamano and Linus Torvalds
*
* Clone a repository into a different directory that does not yet exist.
*/
#define USE_THE_INDEX_COMPATIBILITY_MACROS
#include "builtin.h"
#include "config.h"
#include "lockfile.h"
#include "parse-options.h"
#include "fetch-pack.h"
#include "refs.h"
#include "refspec.h"
#include "object-store.h"
#include "tree.h"
#include "tree-walk.h"
#include "unpack-trees.h"
#include "transport.h"
#include "strbuf.h"
#include "dir.h"
#include "dir-iterator.h"
#include "iterator.h"
#include "sigchain.h"
#include "branch.h"
#include "remote.h"
#include "run-command.h"
#include "connected.h"
#include "packfile.h"
#include "list-objects-filter-options.h"
-#include "object-store.h"
/*
* Overall FIXMEs:
* - respect DB_ENVIRONMENT for .git/objects.
*
* Implementation notes:
* - dropping use-separate-remote and no-separate-remote compatibility
*
*/
diff --git a/builtin/describe.c b/builtin/describe.c
index e048f85484..90feab1120 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -1,22 +1,21 @@
#define USE_THE_INDEX_COMPATIBILITY_MACROS
#include "cache.h"
#include "config.h"
#include "lockfile.h"
#include "commit.h"
#include "tag.h"
#include "blob.h"
#include "refs.h"
#include "builtin.h"
#include "exec-cmd.h"
#include "parse-options.h"
#include "revision.h"
#include "diff.h"
#include "hashmap.h"
#include "argv-array.h"
#include "run-command.h"
#include "object-store.h"
-#include "revision.h"
#include "list-objects.h"
#include "commit-slab.h"
#define MAX_TAGS (FLAG_BITS - 1)
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index b8dc2e1fba..fb8187fba5 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -1,24 +1,23 @@
#include "cache.h"
#include "config.h"
#include "commit.h"
#include "diff.h"
#include "revision.h"
#include "list-objects.h"
#include "list-objects-filter.h"
#include "list-objects-filter-options.h"
#include "object.h"
#include "object-store.h"
#include "pack.h"
#include "pack-bitmap.h"
#include "builtin.h"
#include "log-tree.h"
#include "graph.h"
#include "bisect.h"
#include "progress.h"
#include "reflog-walk.h"
#include "oidset.h"
#include "packfile.h"
-#include "object-store.h"
static const char rev_list_usage[] =
"git rev-list [OPTION] ... [ -- paths... ]\n"
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7f094f8170..0a537881
Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts
Hi All,
I', Just reopening this feature request.
A quick summary of my proposal is given below.
1. This PR will allow an additional configuration option
"guitool..gitgui-shortcut" which will allow us to specify
keyboard shortcut for custom commands in git-gui
2. Even there exists a parameter called "guitool..shortcut"
which is used by git-cola, I suggest to keep this new additional
config parameter as an independent config parameter, which will not
interfere with git-cola in any way, because, both are different
applications and it may have different "built-in" shortcuts already
assigned. So, sharing shortcut scheme between two apps is not a good
idea.
3. New parameter will expect shortcut combinations specified in TCL/TK
's format and we will not be doing any processing on it. Will keep it
simple.
---
Documentation/config/guitool.txt | 15 +++
git-gui/lib/tools.tcl| 15 ---
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/Documentation/config/guitool.txt b/Documentation/config/guitool.txt
index 43fb9466ff..79dac23ca3 100644
--- a/Documentation/config/guitool.txt
+++ b/Documentation/config/guitool.txt
@@ -48,3 +48,18 @@ guitool..prompt::
Specifies the general prompt string to display at the top of
the dialog, before subsections for 'argPrompt' and 'revPrompt'.
The default value includes the actual command.
+
+guitool..gitgui-shortcut
+ Specifies a keyboard shortcut for the custom tool in the git-gui
+ application. The value must be a valid string ( without "<" , ">" wrapper )
+ understood by the TCL/TK 's bind command.See
https://www.tcl.tk/man/tcl8.4/TkCmd/bind.htm
+ for more details about the supported values. Avoid creating shortcuts that
+ conflict with existing built-in `git gui` shortcuts.
+ Example:
+ [guitool "Terminal"]
+ cmd = gnome-terminal -e zsh
+ noconsole = yes
+ gitgui-shortcut = "Control-y"
+ [guitool "Sync"]
+ cmd = "git pull; git push"
+ gitgui-shortcut = "Alt-s"
diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl
index 413f1a1700..40db3f6395 100644
--- a/git-gui/lib/tools.tcl
+++ b/git-gui/lib/tools.tcl
@@ -38,7 +38,7 @@ proc tools_create_item {parent args} {
}
proc tools_populate_one {fullname} {
- global tools_menubar tools_menutbl tools_id
+ global tools_menubar tools_menutbl tools_id repo_config
if {![info exists tools_id]} {
set tools_id 0
@@ -61,9 +61,18 @@ proc tools_populate_one {fullname} {
}
}
- tools_create_item $parent command \
+ if {[info exists repo_config(guitool.$fullname.gitgui-shortcut)]} {
+ set gitgui_shortcut $repo_config(guitool.$fullname.gitgui-shortcut)
+ tools_create_item $parent command \
-label [lindex $names end] \
- -command [list tools_exec $fullname]
+ -command [list tools_exec $fullname] \
+ -accelerator $gitgui_shortcut
+ bind . <$gitgui_shortcut> [list tools_exec $fullname]
+ } else {
+ tools_create_item $parent command \
+ -label [lindex $names end] \
+ -command [list tools_exec $fullname]
+ }
}
proc tools_exec {fullname} {
--
https://github.com/git/git/pull/220
--
On Fri, Apr 1, 2016 at 12:02 PM harish k wrote:
>
> Hi David,
>
> Actually Im a TCL primer. This is the first time Im dealing with.
> That is why I kept it simple ( ie both accel-key and accel-label need
> to be defined in config ).
>
> I think, git-cola is using Qt style representation accel-key in the config
> file.
>
> Is git-cola is an official tool from git ? like git-gui?
> if not ,
> My suggesion is, it is better to seperate config fields according to
> application-domain
> like, "git-gui-accel = " etc..
> Other vise there is good chance for conflicts. ( Eg: consider the case
> that, was assined to a custom tool by git-cola )
>
> Currently this patch will not handle any conflicting shortcuts. I
> think custom shortcuts will overwrite the other.
>
>
> On Thu, Mar 31, 2016 at 10:11 PM, David Aguilar wrote:
> > Hello,
> >
> > On Tue, Mar 29, 2016 at 11:38:10AM +, Harish K wrote:
> >> ---
> >> git-gui/lib/tools.tcl | 16 +---
> >> 1 file changed, 13 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl
> >> index 6ec9411..749bc67 100644
> >> --- a/git-gui/lib/tools.tcl
> >> +++ b/git-gui/lib/tools.tcl
> >> @@ -38,7 +38,7 @@ proc tools_create_item {parent args} {
> >> }
> >>
> >> proc tools_populate_one {fullname} {
> >> - global tools_menubar tools_menutbl tools_id
> >> + global tools_menubar tools_menutbl tools_id repo_config
> >>
> >> if {![info exists tools_id]} {
> >> set tools_id 0
> >> @@ -61,9 +61,19 @@ proc tools_populate_one {fullname} {
> >> }
> >> }
> >>
> >> - tools_create_item $parent command \
> >> + if {[info exists repo_config(guitool.$fullname.accelerator)] &&
> >> [info exists repo_config(guitool.$fullname.accelerator-label)]} {
> >> + set accele_key $repo_config(guitool.$fullname.accelerator)
> >> + set accel_label
> >> $repo_config
Re: [PATCH 1/3] format-patch: document and exercise that -o does only create the trailing directory
Denton, On Wed, Oct 2, 2019 at 11:47 PM Denton Liu wrote: > > Hi Bert, > > > Subject: format-patch: document and exercise that -o does only create the > > trailing directory > > s/does only create/only creates/ ? > > Anyway, as a prepatory patch, I don't think that it's necessary. Maybe > it's just me but I assume that most tools create at most one directory > deep. Even mkdir won't created nested dirs unless you pass `-p`. I > dunno. > > On Wed, Oct 02, 2019 at 11:26:11PM +0200, Bert Wesarg wrote: > > Signed-off-by: Bert Wesarg > > --- > > Documentation/config/format.txt| 3 ++- > > Documentation/git-format-patch.txt | 4 +++- > > t/t4014-format-patch.sh| 16 > > 3 files changed, 21 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/config/format.txt > > b/Documentation/config/format.txt > > index 414a5a8a9d..e17c5d6b0f 100644 > > --- a/Documentation/config/format.txt > > +++ b/Documentation/config/format.txt > > @@ -80,7 +80,8 @@ format.coverLetter:: > > > > format.outputDirectory:: > > Set a custom directory to store the resulting files instead of the > > - current working directory. > > + current working directory. Only the trailing directory will be created > > + though. > > > > format.useAutoBase:: > > A boolean value which lets you enable the `--base=auto` option of > > diff --git a/Documentation/git-format-patch.txt > > b/Documentation/git-format-patch.txt > > index b9b97e63ae..fe7492353e 100644 > > --- a/Documentation/git-format-patch.txt > > +++ b/Documentation/git-format-patch.txt > > @@ -66,7 +66,9 @@ they are created in the current working directory. The > > default path > > can be set with the `format.outputDirectory` configuration option. > > The `-o` option takes precedence over `format.outputDirectory`. > > To store patches in the current working directory even when > > -`format.outputDirectory` points elsewhere, use `-o .`. > > +`format.outputDirectory` points elsewhere, use `-o .`. Note that only > > +the trailing directory will be created by Git, leading directories must > > +already exists. > > > > By default, the subject of a single patch is "[PATCH] " followed by > > the concatenation of lines from the commit message up to the first blank > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > > index ca7debf1d4..bf2715a503 100755 > > --- a/t/t4014-format-patch.sh > > +++ b/t/t4014-format-patch.sh > > @@ -1632,6 +1632,22 @@ test_expect_success 'From line has expected format' ' > > test_cmp from filtered > > ' > > > > +test_expect_success 'format-patch -o with no leading directories' ' > > + rm -fr patches && > > + git format-patch -o patches master..side && > > + test $(git rev-list master..side | wc -l) -eq $(ls patches | wc -l) > > For test case you write, please use the following pattern: > > git rev-list master..side >list && > test_line_count = $(ls patches | wc -l) list > > The first benefit is that we get to take advantage of the > test_line_count function that's already written for us. The second is > that when we write tests, we shouldn't put Git commands in the upstream > of a pipe because if they fail, their return codes will be lost and we > won't be able to fail the test properly. thanks. I will ensure, that this follows your dl/format-patch-doc-test-cleanup topic. Bert > > > +' > > + > > +test_expect_success 'format-patch -o with leading existing directories' ' > > + git format-patch -o patches/side master..side && > > + test $(git rev-list master..side | wc -l) -eq $(ls patches/side | wc > > -l) > > +' > > + > > +test_expect_failure 'format-patch -o with leading non-existing > > directories' ' > > + rm -fr patches && > > + git format-patch -o patches/side master..side > > +' > > As above, I wouldn't really call this a bug in Git. I think we should > leave this test case off until the next patch. > > > + > > test_expect_success 'format-patch format.outputDirectory option' ' > > test_config format.outputDirectory patches && > > rm -fr patches && > > -- > > 2.23.0.11.g242cf7f110 > >
[PATCH v3 0/6] multi-pack-index: add --no-progress
Hello Git contributors,
This is the third iteration of changes for adding support for
--[no-]progress to multi-pack-index, and it includes the following updates
for the feedback I received on v2:
* Fixed commit message formatting
* Updated 'pack_paths_checked' from u32 to unsigned
Thanks, William
William Baker (6):
midx: add MIDX_PROGRESS flag
midx: add progress to write_midx_file
midx: add progress to expire_midx_packs
midx: honor the MIDX_PROGRESS flag in verify_midx_file
midx: honor the MIDX_PROGRESS flag in midx_repack
multi-pack-index: add [--[no-]progress] option.
Documentation/git-multi-pack-index.txt | 6 ++-
builtin/multi-pack-index.c | 18 +--
builtin/repack.c | 2 +-
midx.c | 71 --
midx.h | 10 ++--
t/t5319-multi-pack-index.sh| 69 +
6 files changed, 149 insertions(+), 27 deletions(-)
base-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9
Published-As:
https://github.com/gitgitgadget/git/releases/tag/pr-337%2Fwilbaker%2Fmulti-pack-index-progress-toggle-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-337/wilbaker/multi-pack-index-progress-toggle-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/337
Range-diff vs v2:
1: 6badd9ceaf ! 1: cd041107de midx: add MIDX_PROGRESS flag Add the
MIDX_PROGRESS flag and update the write|verify|expire|repack functions in
midx.h to accept a flags parameter. The MIDX_PROGRESS flag indicates whether
the caller of the function would like progress information to be displayed.
This patch only changes the method prototypes and does not change the
functionality. The functionality change will be handled by a later patch.
@@ -1,6 +1,7 @@
Author: William Baker
midx: add MIDX_PROGRESS flag
+
Add the MIDX_PROGRESS flag and update the
write|verify|expire|repack functions in midx.h
to accept a flags parameter. The MIDX_PROGRESS
2: 3bc8677ea7 ! 2: 0117f55c9d midx: add progress to write_midx_file Add
progress to write_midx_file. Progress is displayed when the MIDX_PROGRESS flag
is set.
@@ -1,6 +1,7 @@
Author: William Baker
midx: add progress to write_midx_file
+
Add progress to write_midx_file. Progress is displayed
when the MIDX_PROGRESS flag is set.
@@ -14,7 +15,7 @@
uint32_t alloc;
struct multi_pack_index *m;
+ struct progress *progress;
-+ uint32_t pack_paths_checked;
++ unsigned pack_paths_checked;
};
static void add_pack_to_midx(const char *full_path, size_t full_path_len,
3: 3374574001 ! 3: d741dc60bc midx: add progress to expire_midx_packs Add
progress to expire_midx_packs. Progress is displayed when the MIDX_PROGRESS
flag is set.
@@ -1,6 +1,7 @@
Author: William Baker
midx: add progress to expire_midx_packs
+
Add progress to expire_midx_packs. Progress is
displayed when the MIDX_PROGRESS flag is set.
4: 29d03771c0 ! 4: f208c09639 midx: honor the MIDX_PROGRESS flag in
verify_midx_file Update verify_midx_file to only display progress when the
MIDX_PROGRESS flag is set.
@@ -1,6 +1,7 @@
Author: William Baker
midx: honor the MIDX_PROGRESS flag in verify_midx_file
+
Update verify_midx_file to only display progress when
the MIDX_PROGRESS flag is set.
5: 57f6742f09 ! 5: 7566090769 midx: honor the MIDX_PROGRESS flag in
midx_repack Update midx_repack to only display progress when the MIDX_PROGRESS
flag is set.
@@ -1,6 +1,7 @@
Author: William Baker
midx: honor the MIDX_PROGRESS flag in midx_repack
+
Update midx_repack to only display progress when
the MIDX_PROGRESS flag is set.
6: 5b933ab744 ! 6: a3c50948d9 multi-pack-index: add [--[no-]progress]
option. Add the --[no-]progress option to git multi-pack-index. Pass the
MIDX_PROGRESS flag to the subcommand functions when progress should be
displayed by multi-pack-index. The progress feature was added to 'verify' in
144d703 ("multi-pack-index: report progress during 'verify'", 2018-09-13) but
some subcommands were not updated to display progress, and the ability to
opt-out was overlooked.
@@ -1,6 +1,7 @@
Author: William Baker
multi-pack-index: add [--[no-]progress] option.
+
Add the --[no-]progress option to git multi-pack-index.
Pass the MIDX_PROGRESS flag to the subcommand functions
when progress should be displayed by multi-pack-index.
--
gitgitgadget
[PATCH v3 2/6] midx: add progress to write_midx_file
From: William Baker
Add progress to write_midx_file. Progress is displayed
when the MIDX_PROGRESS flag is set.
Signed-off-by: William Baker
---
midx.c | 25 +
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/midx.c b/midx.c
index b2673f52e8..0808a40dd4 100644
--- a/midx.c
+++ b/midx.c
@@ -449,6 +449,8 @@ struct pack_list {
uint32_t nr;
uint32_t alloc;
struct multi_pack_index *m;
+ struct progress *progress;
+ unsigned pack_paths_checked;
};
static void add_pack_to_midx(const char *full_path, size_t full_path_len,
@@ -457,6 +459,7 @@ static void add_pack_to_midx(const char *full_path, size_t
full_path_len,
struct pack_list *packs = (struct pack_list *)data;
if (ends_with(file_name, ".idx")) {
+ display_progress(packs->progress, ++packs->pack_paths_checked);
if (packs->m && midx_contains_pack(packs->m, file_name))
return;
@@ -786,7 +789,7 @@ static size_t write_midx_large_offsets(struct hashfile *f,
uint32_t nr_large_off
}
static int write_midx_internal(const char *object_dir, struct multi_pack_index
*m,
- struct string_list *packs_to_drop)
+ struct string_list *packs_to_drop, unsigned
flags)
{
unsigned char cur_chunk, num_chunks = 0;
char *midx_name;
@@ -800,6 +803,7 @@ static int write_midx_internal(const char *object_dir,
struct multi_pack_index *
uint64_t chunk_offsets[MIDX_MAX_CHUNKS + 1];
uint32_t nr_entries, num_large_offsets = 0;
struct pack_midx_entry *entries = NULL;
+ struct progress *progress = NULL;
int large_offsets_needed = 0;
int pack_name_concat_len = 0;
int dropped_packs = 0;
@@ -833,8 +837,15 @@ static int write_midx_internal(const char *object_dir,
struct multi_pack_index *
packs.nr++;
}
}
+
+ packs.pack_paths_checked = 0;
+ if (flags & MIDX_PROGRESS)
+ packs.progress = start_progress(_("Adding packfiles to
multi-pack-index"), 0);
+ else
+ packs.progress = NULL;
for_each_file_in_pack_dir(object_dir, add_pack_to_midx, &packs);
+ stop_progress(&packs.progress);
if (packs.m && packs.nr == packs.m->num_packs && !packs_to_drop)
goto cleanup;
@@ -959,6 +970,9 @@ static int write_midx_internal(const char *object_dir,
struct multi_pack_index *
written += MIDX_CHUNKLOOKUP_WIDTH;
}
+ if (flags & MIDX_PROGRESS)
+ progress = start_progress(_("Writing chunks to
multi-pack-index"),
+ num_chunks);
for (i = 0; i < num_chunks; i++) {
if (written != chunk_offsets[i])
BUG("incorrect chunk offset (%"PRIu64" != %"PRIu64")
for chunk id %"PRIx32,
@@ -991,7 +1005,10 @@ static int write_midx_internal(const char *object_dir,
struct multi_pack_index *
BUG("trying to write unknown chunk id %"PRIx32,
chunk_ids[i]);
}
+
+ display_progress(progress, i + 1);
}
+ stop_progress(&progress);
if (written != chunk_offsets[num_chunks])
BUG("incorrect final offset %"PRIu64" != %"PRIu64,
@@ -1019,7 +1036,7 @@ static int write_midx_internal(const char *object_dir,
struct multi_pack_index *
int write_midx_file(const char *object_dir, unsigned flags)
{
- return write_midx_internal(object_dir, NULL, NULL);
+ return write_midx_internal(object_dir, NULL, NULL, flags);
}
void clear_midx_file(struct repository *r)
@@ -1222,7 +1239,7 @@ int expire_midx_packs(struct repository *r, const char
*object_dir, unsigned fla
free(count);
if (packs_to_drop.nr)
- result = write_midx_internal(object_dir, m, &packs_to_drop);
+ result = write_midx_internal(object_dir, m, &packs_to_drop,
flags);
string_list_clear(&packs_to_drop, 0);
return result;
@@ -1371,7 +1388,7 @@ int midx_repack(struct repository *r, const char
*object_dir, size_t batch_size,
goto cleanup;
}
- result = write_midx_internal(object_dir, m, NULL);
+ result = write_midx_internal(object_dir, m, NULL, flags);
m = NULL;
cleanup:
--
gitgitgadget
[PATCH v3 5/6] midx: honor the MIDX_PROGRESS flag in midx_repack
From: William Baker Update midx_repack to only display progress when the MIDX_PROGRESS flag is set. Signed-off-by: William Baker --- midx.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/midx.c b/midx.c index ced5898bbf..1c5ddeb007 100644 --- a/midx.c +++ b/midx.c @@ -1374,6 +1374,12 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, strbuf_addstr(&base_name, object_dir); strbuf_addstr(&base_name, "/pack/pack"); argv_array_push(&cmd.args, base_name.buf); + + if (flags & MIDX_PROGRESS) + argv_array_push(&cmd.args, "--progress"); + else + argv_array_push(&cmd.args, "-q"); + strbuf_release(&base_name); cmd.git_cmd = 1; -- gitgitgadget
[PATCH v3 6/6] multi-pack-index: add [--[no-]progress] option.
From: William Baker
Add the --[no-]progress option to git multi-pack-index.
Pass the MIDX_PROGRESS flag to the subcommand functions
when progress should be displayed by multi-pack-index.
The progress feature was added to 'verify' in 144d703
("multi-pack-index: report progress during 'verify'", 2018-09-13)
but some subcommands were not updated to display progress, and
the ability to opt-out was overlooked.
Signed-off-by: William Baker
---
Documentation/git-multi-pack-index.txt | 6 ++-
builtin/multi-pack-index.c | 18 +--
t/t5319-multi-pack-index.sh| 69 ++
3 files changed, 87 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-multi-pack-index.txt
b/Documentation/git-multi-pack-index.txt
index 233b2b7862..d7a02cc6fa 100644
--- a/Documentation/git-multi-pack-index.txt
+++ b/Documentation/git-multi-pack-index.txt
@@ -9,7 +9,7 @@ git-multi-pack-index - Write and verify multi-pack-indexes
SYNOPSIS
[verse]
-'git multi-pack-index' [--object-dir=]
+'git multi-pack-index' [--object-dir=] [--[no-]progress]
DESCRIPTION
---
@@ -23,6 +23,10 @@ OPTIONS
`/packs/multi-pack-index` for the current MIDX file, and
`/packs` for the pack-files to index.
+--[no-]progress::
+ Turn progress on/off explicitly. If neither is specified, progress is
+ shown if standard error is connected to a terminal.
+
The following subcommands are available:
write::
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index e86b8cd17d..1730b21901 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -6,21 +6,25 @@
#include "trace2.h"
static char const * const builtin_multi_pack_index_usage[] = {
- N_("git multi-pack-index [--object-dir=]
(write|verify|expire|repack --batch-size=)"),
+ N_("git multi-pack-index [] (write|verify|expire|repack
--batch-size=)"),
NULL
};
static struct opts_multi_pack_index {
const char *object_dir;
unsigned long batch_size;
+ int progress;
} opts;
int cmd_multi_pack_index(int argc, const char **argv,
const char *prefix)
{
+ unsigned flags = 0;
+
static struct option builtin_multi_pack_index_options[] = {
OPT_FILENAME(0, "object-dir", &opts.object_dir,
N_("object directory containing set of packfile and
pack-index pairs")),
+ OPT_BOOL(0, "progress", &opts.progress, N_("force progress
reporting")),
OPT_MAGNITUDE(0, "batch-size", &opts.batch_size,
N_("during repack, collect pack-files of smaller size into a
batch that is larger than this size")),
OPT_END(),
@@ -28,12 +32,15 @@ int cmd_multi_pack_index(int argc, const char **argv,
git_config(git_default_config, NULL);
+ opts.progress = isatty(2);
argc = parse_options(argc, argv, prefix,
builtin_multi_pack_index_options,
builtin_multi_pack_index_usage, 0);
if (!opts.object_dir)
opts.object_dir = get_object_directory();
+ if (opts.progress)
+ flags |= MIDX_PROGRESS;
if (argc == 0)
usage_with_options(builtin_multi_pack_index_usage,
@@ -47,16 +54,17 @@ int cmd_multi_pack_index(int argc, const char **argv,
trace2_cmd_mode(argv[0]);
if (!strcmp(argv[0], "repack"))
- return midx_repack(the_repository, opts.object_dir,
(size_t)opts.batch_size, 0);
+ return midx_repack(the_repository, opts.object_dir,
+ (size_t)opts.batch_size, flags);
if (opts.batch_size)
die(_("--batch-size option is only for 'repack' subcommand"));
if (!strcmp(argv[0], "write"))
- return write_midx_file(opts.object_dir, 0);
+ return write_midx_file(opts.object_dir, flags);
if (!strcmp(argv[0], "verify"))
- return verify_midx_file(the_repository, opts.object_dir, 0);
+ return verify_midx_file(the_repository, opts.object_dir, flags);
if (!strcmp(argv[0], "expire"))
- return expire_midx_packs(the_repository, opts.object_dir, 0);
+ return expire_midx_packs(the_repository, opts.object_dir,
flags);
die(_("unrecognized subcommand: %s"), argv[0]);
}
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index c72ca04399..cd2f87be6a 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -147,6 +147,21 @@ test_expect_success 'write midx with two packs' '
compare_results_with_midx "two packs"
+test_expect_success 'write progress off for redirected stderr' '
+ git multi-pack-index --object-dir=$objdir write 2>err &&
+ test_line_count = 0 err
+'
+
+test_expect_success 'write force progress on for stderr' '
+ git multi-pack-index --object-dir=
[PATCH v3 1/6] midx: add MIDX_PROGRESS flag
From: William Baker
Add the MIDX_PROGRESS flag and update the
write|verify|expire|repack functions in midx.h
to accept a flags parameter. The MIDX_PROGRESS
flag indicates whether the caller of the function
would like progress information to be displayed.
This patch only changes the method prototypes
and does not change the functionality. The
functionality change will be handled by a later patch.
Signed-off-by: William Baker
---
builtin/multi-pack-index.c | 8
builtin/repack.c | 2 +-
midx.c | 8
midx.h | 10 ++
4 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index b1ea1a6aa1..e86b8cd17d 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -47,16 +47,16 @@ int cmd_multi_pack_index(int argc, const char **argv,
trace2_cmd_mode(argv[0]);
if (!strcmp(argv[0], "repack"))
- return midx_repack(the_repository, opts.object_dir,
(size_t)opts.batch_size);
+ return midx_repack(the_repository, opts.object_dir,
(size_t)opts.batch_size, 0);
if (opts.batch_size)
die(_("--batch-size option is only for 'repack' subcommand"));
if (!strcmp(argv[0], "write"))
- return write_midx_file(opts.object_dir);
+ return write_midx_file(opts.object_dir, 0);
if (!strcmp(argv[0], "verify"))
- return verify_midx_file(the_repository, opts.object_dir);
+ return verify_midx_file(the_repository, opts.object_dir, 0);
if (!strcmp(argv[0], "expire"))
- return expire_midx_packs(the_repository, opts.object_dir);
+ return expire_midx_packs(the_repository, opts.object_dir, 0);
die(_("unrecognized subcommand: %s"), argv[0]);
}
diff --git a/builtin/repack.c b/builtin/repack.c
index 632c0c0a79..5b3623337f 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -561,7 +561,7 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
remove_temporary_files();
if (git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0))
- write_midx_file(get_object_directory());
+ write_midx_file(get_object_directory(), 0);
string_list_clear(&names, 0);
string_list_clear(&rollback, 0);
diff --git a/midx.c b/midx.c
index d649644420..b2673f52e8 100644
--- a/midx.c
+++ b/midx.c
@@ -1017,7 +1017,7 @@ static int write_midx_internal(const char *object_dir,
struct multi_pack_index *
return result;
}
-int write_midx_file(const char *object_dir)
+int write_midx_file(const char *object_dir, unsigned flags)
{
return write_midx_internal(object_dir, NULL, NULL);
}
@@ -1077,7 +1077,7 @@ static int compare_pair_pos_vs_id(const void *_a, const
void *_b)
display_progress(progress, _n); \
} while (0)
-int verify_midx_file(struct repository *r, const char *object_dir)
+int verify_midx_file(struct repository *r, const char *object_dir, unsigned
flags)
{
struct pair_pos_vs_id *pairs = NULL;
uint32_t i;
@@ -1184,7 +1184,7 @@ int verify_midx_file(struct repository *r, const char
*object_dir)
return verify_midx_error;
}
-int expire_midx_packs(struct repository *r, const char *object_dir)
+int expire_midx_packs(struct repository *r, const char *object_dir, unsigned
flags)
{
uint32_t i, *count, result = 0;
struct string_list packs_to_drop = STRING_LIST_INIT_DUP;
@@ -1316,7 +1316,7 @@ static int fill_included_packs_batch(struct repository *r,
return 0;
}
-int midx_repack(struct repository *r, const char *object_dir, size_t
batch_size)
+int midx_repack(struct repository *r, const char *object_dir, size_t
batch_size, unsigned flags)
{
int result = 0;
uint32_t i;
diff --git a/midx.h b/midx.h
index f0ae656b5d..e6fa356b5c 100644
--- a/midx.h
+++ b/midx.h
@@ -37,6 +37,8 @@ struct multi_pack_index {
char object_dir[FLEX_ARRAY];
};
+#define MIDX_PROGRESS (1 << 0)
+
struct multi_pack_index *load_multi_pack_index(const char *object_dir, int
local);
int prepare_midx_pack(struct repository *r, struct multi_pack_index *m,
uint32_t pack_int_id);
int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m,
uint32_t *result);
@@ -47,11 +49,11 @@ int fill_midx_entry(struct repository *r, const struct
object_id *oid, struct pa
int midx_contains_pack(struct multi_pack_index *m, const char
*idx_or_pack_name);
int prepare_multi_pack_index_one(struct repository *r, const char *object_dir,
int local);
-int write_midx_file(const char *object_dir);
+int write_midx_file(const char *object_dir, unsigned flags);
void clear_midx_file(struct repository *r);
-int verify_midx_file(struct repository *r, const char *object_dir);
-int expire_midx_packs(struct repository *r, const char *object_dir);
-int midx_repack(struct repository *r,
[PATCH v3 4/6] midx: honor the MIDX_PROGRESS flag in verify_midx_file
From: William Baker
Update verify_midx_file to only display progress when
the MIDX_PROGRESS flag is set.
Signed-off-by: William Baker
---
midx.c | 20
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/midx.c b/midx.c
index f14bebb092..ced5898bbf 100644
--- a/midx.c
+++ b/midx.c
@@ -1098,15 +1098,16 @@ int verify_midx_file(struct repository *r, const char
*object_dir, unsigned flag
{
struct pair_pos_vs_id *pairs = NULL;
uint32_t i;
- struct progress *progress;
+ struct progress *progress = NULL;
struct multi_pack_index *m = load_multi_pack_index(object_dir, 1);
verify_midx_error = 0;
if (!m)
return 0;
- progress = start_progress(_("Looking for referenced packfiles"),
- m->num_packs);
+ if (flags & MIDX_PROGRESS)
+ progress = start_progress(_("Looking for referenced packfiles"),
+ m->num_packs);
for (i = 0; i < m->num_packs; i++) {
if (prepare_midx_pack(r, m, i))
midx_report("failed to load pack in position %d", i);
@@ -1124,8 +1125,9 @@ int verify_midx_file(struct repository *r, const char
*object_dir, unsigned flag
i, oid_fanout1, oid_fanout2, i + 1);
}
- progress = start_sparse_progress(_("Verifying OID order in MIDX"),
-m->num_objects - 1);
+ if (flags & MIDX_PROGRESS)
+ progress = start_sparse_progress(_("Verifying OID order in
multi-pack-index"),
+m->num_objects - 1);
for (i = 0; i < m->num_objects - 1; i++) {
struct object_id oid1, oid2;
@@ -1152,13 +1154,15 @@ int verify_midx_file(struct repository *r, const char
*object_dir, unsigned flag
pairs[i].pack_int_id = nth_midxed_pack_int_id(m, i);
}
- progress = start_sparse_progress(_("Sorting objects by packfile"),
-m->num_objects);
+ if (flags & MIDX_PROGRESS)
+ progress = start_sparse_progress(_("Sorting objects by
packfile"),
+m->num_objects);
display_progress(progress, 0); /* TODO: Measure QSORT() progress */
QSORT(pairs, m->num_objects, compare_pair_pos_vs_id);
stop_progress(&progress);
- progress = start_sparse_progress(_("Verifying object offsets"),
m->num_objects);
+ if (flags & MIDX_PROGRESS)
+ progress = start_sparse_progress(_("Verifying object offsets"),
m->num_objects);
for (i = 0; i < m->num_objects; i++) {
struct object_id oid;
struct pack_entry e;
--
gitgitgadget
[PATCH v3 3/6] midx: add progress to expire_midx_packs
From: William Baker
Add progress to expire_midx_packs. Progress is
displayed when the MIDX_PROGRESS flag is set.
Signed-off-by: William Baker
---
midx.c | 12
1 file changed, 12 insertions(+)
diff --git a/midx.c b/midx.c
index 0808a40dd4..f14bebb092 100644
--- a/midx.c
+++ b/midx.c
@@ -1206,18 +1206,29 @@ int expire_midx_packs(struct repository *r, const char
*object_dir, unsigned fla
uint32_t i, *count, result = 0;
struct string_list packs_to_drop = STRING_LIST_INIT_DUP;
struct multi_pack_index *m = load_multi_pack_index(object_dir, 1);
+ struct progress *progress = NULL;
if (!m)
return 0;
count = xcalloc(m->num_packs, sizeof(uint32_t));
+
+ if (flags & MIDX_PROGRESS)
+ progress = start_progress(_("Counting referenced objects"),
+ m->num_objects);
for (i = 0; i < m->num_objects; i++) {
int pack_int_id = nth_midxed_pack_int_id(m, i);
count[pack_int_id]++;
+ display_progress(progress, i + 1);
}
+ stop_progress(&progress);
+ if (flags & MIDX_PROGRESS)
+ progress = start_progress(_("Finding and deleting unreferenced
packfiles"),
+ m->num_packs);
for (i = 0; i < m->num_packs; i++) {
char *pack_name;
+ display_progress(progress, i + 1);
if (count[i])
continue;
@@ -1235,6 +1246,7 @@ int expire_midx_packs(struct repository *r, const char
*object_dir, unsigned fla
unlink_pack_path(pack_name, 0);
free(pack_name);
}
+ stop_progress(&progress);
free(count);
--
gitgitgadget
[no subject]
subscribe me
Re: [RFC PATCH v5 2/3] grep: make PCRE2 aware of custom allocator
Hi Carlo, On Thu, 3 Oct 2019, Carlo Arenas wrote: > On Thu, Oct 3, 2019 at 1:09 AM Johannes Schindelin > wrote: > > I still need > > https://github.com/git-for-windows/git/commit/719beb813e4f27f090696ce583df3e5f3c480545 > > and > > https://github.com/git-for-windows/git/commit/3369c322bbd95820b971701fef7db44b26dd826f > > to fix that part in Git for Windows' `shears/pu` branch (i.e. the > > continuously rebased patch thicket). > > or we could drop the current branch in pu and start again from scratch > now that all of the required dependencies had been merged. I hope that your next iteration won't have any `#ifdef NED` in it? > > apologize for the delays otherwise; $DAYJOB has taken a toll lately > and even my new shiny windows dev box hasn't seen much action. > > will update here and in github shortly (which might mean up to this > weekend, being realistic), but should be better code (since it is > mostly Rene's) and better tested that way and hopefully won't cause > more breakage (specially in Windows, sorry Dscho) I appreciate all your work! Thanks, Dscho
Re: What's cooking in git.git (Oct 2019, #01; Thu, 3)
On Thu, Oct 03, 2019 at 02:04:39PM +0900, Junio C Hamano wrote: > Here are the topics that have been cooking. Commits prefixed with > '-' are only in 'pu' (proposed updates) while commits prefixed with > '+' are in 'next'. The ones marked with '.' do not appear in any of > the integration branches, but I am still holding onto them. > > You can find the changes described here in the integration branches > of the repositories listed at > > http://git-blame.blogspot.com/p/git-public-repositories.html > Hi Junio, How do you feel about https://public-inbox.org/git/20190930220355.108394-1-emilyshaf...@google.com/ which fixes a user-facing SIGABRT during partial clone operations? It's reviewed by Peff and Christian. - Emily
[PATCH 1/1] fsmonitor: don't fill bitmap with entries to be removed
From: William Baker
While doing some testing with fsmonitor enabled I found
that git commands would segfault after staging and
unstaging an untracked file. Looking at the crash it
appeared that fsmonitor_ewah_callback was attempting to
adjust bits beyond the bounds of the index cache.
Digging into how this could happen it became clear that
the fsmonitor extension must have been written with
more bits than there were entries in the index. The
root cause ended up being that fill_fsmonitor_bitmap was
populating fsmonitor_dirty with bits for all entries in
the index, even those that had been marked for removal.
To solve this problem fill_fsmonitor_bitmap has been
updated to skip entries with the the CE_REMOVE flag set.
With this change the bits written for the fsmonitor
extension will be consistent with the index entries
written by do_write_index. Additionally, BUG checks
have been added to detect if the number of bits in
fsmonitor_dirty should ever exceed the number of
entries in the index again.
Another option that was considered was moving the call
to fill_fsmonitor_bitmap closer to where the index is
written (and where the fsmonitor extension itself is
written). However, that did not work as the
fsmonitor_dirty bitmap must be filled before the index
is split during writing.
Signed-off-by: William Baker
---
fsmonitor.c | 29 -
t/t7519-status-fsmonitor.sh | 12
t/t7519/fsmonitor-env | 24
3 files changed, 60 insertions(+), 5 deletions(-)
create mode 100755 t/t7519/fsmonitor-env
diff --git a/fsmonitor.c b/fsmonitor.c
index 231e83a94d..fa0b96d84e 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -14,8 +14,13 @@ struct trace_key trace_fsmonitor = TRACE_KEY_INIT(FSMONITOR);
static void fsmonitor_ewah_callback(size_t pos, void *is)
{
struct index_state *istate = (struct index_state *)is;
- struct cache_entry *ce = istate->cache[pos];
+ struct cache_entry *ce;
+
+ if (pos >= istate->cache_nr)
+ BUG("fsmonitor_dirty has more entries than the index
(%"PRIuMAX" >= %"PRIuMAX")",
+ (uintmax_t)pos, (uintmax_t)istate->cache_nr);
+ ce = istate->cache[pos];
ce->ce_flags &= ~CE_FSMONITOR_VALID;
}
@@ -50,17 +55,24 @@ int read_fsmonitor_extension(struct index_state *istate,
const void *data,
}
istate->fsmonitor_dirty = fsmonitor_dirty;
+ if (istate->fsmonitor_dirty->bit_size > istate->cache_nr)
+ BUG("fsmonitor_dirty has more entries than the index
(%"PRIuMAX" > %"PRIuMAX")",
+ (uintmax_t)istate->fsmonitor_dirty->bit_size,
(uintmax_t)istate->cache_nr);
+
trace_printf_key(&trace_fsmonitor, "read fsmonitor extension
successful");
return 0;
}
void fill_fsmonitor_bitmap(struct index_state *istate)
{
- unsigned int i;
+ unsigned int i, skipped = 0;
istate->fsmonitor_dirty = ewah_new();
- for (i = 0; i < istate->cache_nr; i++)
- if (!(istate->cache[i]->ce_flags & CE_FSMONITOR_VALID))
- ewah_set(istate->fsmonitor_dirty, i);
+ for (i = 0; i < istate->cache_nr; i++) {
+ if (istate->cache[i]->ce_flags & CE_REMOVE)
+ skipped++;
+ else if (!(istate->cache[i]->ce_flags & CE_FSMONITOR_VALID))
+ ewah_set(istate->fsmonitor_dirty, i - skipped);
+ }
}
void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate)
@@ -71,6 +83,10 @@ void write_fsmonitor_extension(struct strbuf *sb, struct
index_state *istate)
uint32_t ewah_size = 0;
int fixup = 0;
+ if (istate->fsmonitor_dirty->bit_size > istate->cache_nr)
+ BUG("fsmonitor_dirty has more entries than the index
(%"PRIuMAX" > %"PRIuMAX")",
+ (uintmax_t)istate->fsmonitor_dirty->bit_size,
(uintmax_t)istate->cache_nr);
+
put_be32(&hdr_version, INDEX_EXTENSION_VERSION);
strbuf_add(sb, &hdr_version, sizeof(uint32_t));
@@ -236,6 +252,9 @@ void tweak_fsmonitor(struct index_state *istate)
}
/* Mark all previously saved entries as dirty */
+ if (istate->fsmonitor_dirty->bit_size >
istate->cache_nr)
+ BUG("fsmonitor_dirty has more entries than the
index (%"PRIuMAX" > %"PRIuMAX")",
+
(uintmax_t)istate->fsmonitor_dirty->bit_size, (uintmax_t)istate->cache_nr);
ewah_each_bit(istate->fsmonitor_dirty,
fsmonitor_ewah_callback, istate);
/* Now mark the untracked cache for fsmonitor usage */
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 81a375fa0f..85df54f07c 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -354,4 +354,16 @@ test_expect_success 'discard_index() also disca
[PATCH 0/1] fsmonitor: don't fill bitmap with entries to be removed
This patch series fixes a segfault that I encountered while testing fsmonitor. Under some circumstances, the fsmonitor extension was being written with too many bits, and subsequent git commands would segfault when trying to apply the bits to the index. As part of these changes I've added some BUG checks that would have helped catch this problem sooner. Special thanks to Dscho for pointing me in the right direction and suggesting a test that can reproduce the issue. Thanks, William William Baker (1): fsmonitor: don't fill bitmap with entries to be removed fsmonitor.c | 29 - t/t7519-status-fsmonitor.sh | 12 t/t7519/fsmonitor-env | 24 3 files changed, 60 insertions(+), 5 deletions(-) create mode 100755 t/t7519/fsmonitor-env base-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-372%2Fwilbaker%2Ffix_git_fsmonitor_crash-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-372/wilbaker/fix_git_fsmonitor_crash-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/372 -- gitgitgadget
Re: [PATCH 2/2] git-gui: support for diff3 conflict style
On 30/09/2019 13:17, Bert Wesarg wrote:
Pratyush,
On Sun, Sep 29, 2019 at 5:04 PM Pratyush Yadav wrote:
Hi Philip, Bert,
Is there any way I can test this change? Philip, I ran the rebase you
mention in the GitHub issue [0], and I get that '9c8cba6862abe5ac821' is
an unknown revision.
Is there any quick way I can reproduce this (maybe on a sample repo)?
The easiest way to produce a merge conflict, is to change the same
line differently in two branches and try to merge them. I added a
fast-import file to demonstrate this for you.
$ git init foo
$ cd foo
$ git fast-import <../conflict-merge.fi
$ git reset --hard master
$ git merge branch
this gets you into the conflict, probably the usual style. Which looks
in liek this on the terminal:
@@@ -2,7 -2,7 +2,11 @@@ Lorem ipsum dolor sit amet, consectetu
Sed feugiat nisl eget efficitur ultrices.
Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi.
Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra.
++<<< HEAD
+Proin bibendum purus ut est tristique, non pharetra dui consectetur.
++===
+ Proin placerat leo malesuada lacinia lobortis.
++>>> branch
Pellentesque aliquam libero et nisi scelerisque commodo.
Quisque id velit sed magna molestie porttitor.
Vivamus sed urna in risus molestie ultricies.
and this in git gui: https://kgab.selfhost.eu/s/gHHaQqowGp7mXEb
The snapshot of the Gui looks just the thing! (I've been away).
I'm sure this would solve my immediate issue.
My only remaining bikeshed question it prompted was to check which parts
would be committed as part of committing the whole "hunk". But haven't
had time to look at all!
Git gui removes the '++' in front of the marker lines. It therefor
already 'changes' the 'diff'. Though git-apply cannot handle such
'diffs' anyway.
To get the diff3 style do:
$ git merge --abort
$ git -c merge.conflictStyle=diff3 merge branch
This is how it looks in the terminal now:
@@@ -2,7 -2,7 +2,13 @@@ Lorem ipsum dolor sit amet, consectetu
Sed feugiat nisl eget efficitur ultrices.
Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi.
Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra.
++<<< HEAD
+Proin bibendum purus ut est tristique, non pharetra dui consectetur.
++||| merged common ancestors
++Proin in felis eu elit suscipit rhoncus vel ut metus.
++===
+ Proin placerat leo malesuada lacinia lobortis.
++>>> branch
Pellentesque aliquam libero et nisi scelerisque commodo.
Quisque id velit sed magna molestie porttitor.
Vivamus sed urna in risus molestie ultricies.
As you can see, there is not the usual 'I removed this, and added
that' experience, everything is 'added'. Thus I inverted the pre-image
to '--'. Here is how it looks in the gui:
https://kgab.selfhost.eu/s/5c8Tosra7WRfjwJ
[0] https://github.com/git-for-windows/git/issues/2340
On 25/09/19 10:38PM, Bert Wesarg wrote:
This adds highlight support for the diff3 conflict style.
The common pre-image will be reversed to --, because it has been removed
and either replaced with ours or theirs side.
Signed-off-by: Bert Wesarg
---
git-gui.sh | 3 +++
lib/diff.tcl | 22 ++
2 files changed, 25 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index fd476b6..6d80f82 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3581,6 +3581,9 @@ $ui_diff tag conf d_s- \
$ui_diff tag conf d< \
-foreground orange \
-font font_diffbold
+$ui_diff tag conf d| \
+ -foreground orange \
+ -font font_diffbold
$ui_diff tag conf d= \
-foreground orange \
-font font_diffbold
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 0fd4600..6caf4e7 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -347,6 +347,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
}
set ::current_diff_inheader 1
+ set ::conflict_state {CONTEXT}
fconfigure $fd \
-blocking 0 \
-encoding [get_path_encoding $path] \
@@ -450,10 +451,28 @@ proc read_diff {fd conflict_size cont_info} {
{++} {
set regexp [string map [list %conflict_size
$conflict_size]\
{^\+\+([<>=]){%conflict_size}(?: |$)}]
+ set regexp_pre_image [string map [list
%conflict_size $conflict_size]\
+
{^\+\+\|{%conflict_size}(?: |$)}]
if {[regexp $regexp $line _g op]} {
set is_conflict_diff 1
set line [string replace $line 0 1 { }]
+ set markup {}
set tags d$op
+ switch -exact -- $op {
+ < { set ::conflict_state {OURS} }
+ = {
Re: subscribing to list.
On 03/10/2019 19:02, Rohit Sanjay wrote: subscribe me try http://vger.kernel.org/vger-lists.html#git for more info... (email to majord...@vger.kernel.org body = `subscribe git`) -- Philip PS. the list uses bottom posting https://en.wikipedia.org/wiki/Posting_style
[PATCH -v3 0/8] fast export/import: handle nested tags, improve incremental exports
This series improves the incremental export story for fast-export and
fast-import (--export-marks and --import-marks fell a bit short),
fixes a couple small export/import bugs, and enables handling nested
tags. In particular, the nested tags handling makes it so that
fast-export and fast-import can finally handle the git.git repo.
Changes since v2 (full range-diff below):
- Code cleanup of patch 2 suggested by René
Elijah Newren (8):
fast-export: fix exporting a tag and nothing else
fast-import: fix handling of deleted tags
fast-import: allow tags to be identified by mark labels
fast-import: add support for new 'alias' command
fast-export: add support for --import-marks-if-exists
fast-export: allow user to request tags be marked with --mark-tags
t9350: add tests for tags of things other than a commit
fast-export: handle nested tags
Documentation/git-fast-export.txt | 17 --
Documentation/git-fast-import.txt | 23
builtin/fast-export.c | 67 --
fast-import.c | 92 +++
t/t9300-fast-import.sh| 37 +
t/t9350-fast-export.sh| 68 +--
6 files changed, 266 insertions(+), 38 deletions(-)
Range-diff:
1: a30cfbbb50 = 1: a30cfbbb50 fast-export: fix exporting a tag and nothing
else
2: 1d19498bc6 ! 2: 36fbf15134 fast-import: fix handling of deleted tags
@@ Commit message
So, either way nested tags imply the need to delete temporary inner tag
references.
+Helped-by: René Scharfe
Signed-off-by: Elijah Newren
## fast-import.c ##
+@@ fast-import.c: static void parse_new_tag(const char *arg)
+ static void parse_reset_branch(const char *arg)
+ {
+ struct branch *b;
++ const char *tag_name;
+
+ b = lookup_branch(arg);
+ if (b) {
@@ fast-import.c: static void parse_reset_branch(const char *arg)
b = new_branch(arg);
read_next_command();
parse_from(b);
-+ if (b->delete && !strncmp(b->name, "refs/tags/", 10)) {
++ if (b->delete && skip_prefix(b->name, "refs/tags/", &tag_name)) {
+ /*
+ * Elsewhere, we call dump_branches() before dump_tags(),
+ * and dump_branches() will handle ref deletions first, so
@@ fast-import.c: static void parse_reset_branch(const char *arg)
+ * NEEDSWORK: replace list of tags with hashmap for faster
+ * deletion?
+ */
-+ struct strbuf tag_name = STRBUF_INIT;
+ struct tag *t, *prev = NULL;
+ for (t = first_tag; t; t = t->next_tag) {
-+ strbuf_reset(&tag_name);
-+ strbuf_addf(&tag_name, "refs/tags/%s", t->name);
-+ if (!strcmp(b->name, tag_name.buf))
++ if (!strcmp(t->name, tag_name))
+ break;
+ prev = t;
+ }
3: e1fd888e4a = 3: 3b5f4270f8 fast-import: allow tags to be identified by
mark labels
4: 93175f28d9 = 4: 489c7fd854 fast-import: add support for new 'alias' command
5: 8c8743395c = 5: 38fd19caee fast-export: add support for
--import-marks-if-exists
6: eebc40df33 = 6: 2017b8d9f9 fast-export: allow user to request tags be
marked with --mark-tags
7: de39f703c6 = 7: 0efdbb81b1 t9350: add tests for tags of things other than
a commit
8: ac739dbb79 = 8: fe7c27d786 fast-export: handle nested tags
--
2.23.0.264.g3b9f7f2fc6
[PATCH -v3 1/8] fast-export: fix exporting a tag and nothing else
fast-export allows specifying revision ranges, which can be used to
export a tag without exporting the commit it tags. fast-export handled
this rather poorly: it would emit a "from :0" directive. Since marks
start at 1 and increase, this means it refers to an unknown commit and
fast-import will choke on the input.
When we are unable to look up a mark for the object being tagged, use a
"from $HASH" directive instead to fix this problem.
Note that this is quite similar to the behavior fast-export exhibits
with commits and parents when --reference-excluded-parents is passed
along with an excluded commit range. For tags of excluded commits we do
not require the --reference-excluded-parents flag because we always have
to tag something. By contrast, when dealing with commits, pruning a
parent is always a viable option, so we need the flag to specify that
parent pruning is not wanted. (It is slightly weird that
--reference-excluded-parents isn't the default with a separate
--prune-excluded-parents flag, but backward compatibility concerns
resulted in the current defaults.)
Signed-off-by: Elijah Newren
---
builtin/fast-export.c | 7 ++-
t/t9350-fast-export.sh | 13 +
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index f541f55d33..5822271c6b 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -860,7 +860,12 @@ static void handle_tag(const char *name, struct tag *tag)
if (starts_with(name, "refs/tags/"))
name += 10;
- printf("tag %s\nfrom :%d\n", name, tagged_mark);
+ printf("tag %s\n", name);
+ if (tagged_mark)
+ printf("from :%d\n", tagged_mark);
+ else
+ printf("from %s\n", oid_to_hex(&tagged->oid));
+
if (show_original_ids)
printf("original-oid %s\n", oid_to_hex(&tag->object.oid));
printf("%.*s%sdata %d\n%.*s\n",
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index b4004e05c2..d32ff41859 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -53,6 +53,19 @@ test_expect_success 'fast-export | fast-import' '
'
+test_expect_success 'fast-export ^muss^{commit} muss' '
+ git fast-export --tag-of-filtered-object=rewrite ^muss^{commit} muss
>actual &&
+ cat >expected <<-EOF &&
+ tag muss
+ from $(git rev-parse --verify muss^{commit})
+ $(git cat-file tag muss | grep tagger)
+ data 9
+ valentin
+
+ EOF
+ test_cmp expected actual
+'
+
test_expect_success 'fast-export master~2..master' '
git fast-export master~2..master >actual &&
--
2.23.0.264.g3b9f7f2fc6
[PATCH -v3 2/8] fast-import: fix handling of deleted tags
If our input stream includes a tag which is later deleted, we were not
properly deleting it. We did have a step which would delete it, but we
left a tag in the tag list noting that it needed to be updated, and the
updating of annotated tags occurred AFTER ref deletion. So, when we
record that a tag needs to be deleted, also remove it from the list of
annotated tags to update.
While this has likely been something that has not happened in practice,
it will come up more in order to support nested tags. For nested tags,
we either need to give temporary names to the intermediate tags and then
delete them, or else we need to use the final name for the intermediate
tags. If we use the final name for the intermediate tags, then in order
to keep the sanity check that someone doesn't try to update the same tag
twice, we need to delete the ref after creating the intermediate tag.
So, either way nested tags imply the need to delete temporary inner tag
references.
Helped-by: René Scharfe
Signed-off-by: Elijah Newren
---
fast-import.c | 27 +++
t/t9300-fast-import.sh | 13 +
2 files changed, 40 insertions(+)
diff --git a/fast-import.c b/fast-import.c
index b44d6a467e..caae0819f5 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2778,6 +2778,7 @@ static void parse_new_tag(const char *arg)
static void parse_reset_branch(const char *arg)
{
struct branch *b;
+ const char *tag_name;
b = lookup_branch(arg);
if (b) {
@@ -2793,6 +2794,32 @@ static void parse_reset_branch(const char *arg)
b = new_branch(arg);
read_next_command();
parse_from(b);
+ if (b->delete && skip_prefix(b->name, "refs/tags/", &tag_name)) {
+ /*
+* Elsewhere, we call dump_branches() before dump_tags(),
+* and dump_branches() will handle ref deletions first, so
+* in order to make sure the deletion actually takes effect,
+* we need to remove the tag from our list of tags to update.
+*
+* NEEDSWORK: replace list of tags with hashmap for faster
+* deletion?
+*/
+ struct tag *t, *prev = NULL;
+ for (t = first_tag; t; t = t->next_tag) {
+ if (!strcmp(t->name, tag_name))
+ break;
+ prev = t;
+ }
+ if (t) {
+ if (prev)
+ prev->next_tag = t->next_tag;
+ else
+ first_tag = t->next_tag;
+ if (!t->next_tag)
+ last_tag = prev;
+ /* There is no mem_pool_free(t) function to call. */
+ }
+ }
if (command_buf.len > 0)
unread_command_buf = 1;
}
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 141b7fa35e..74bc41333b 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -85,6 +85,15 @@ test_expect_success 'A: create pack from stdin' '
An annotated tag that annotates a blob.
EOF
+ tag to-be-deleted
+ from :3
+ data
[PATCH -v3 4/8] fast-import: add support for new 'alias' command
fast-export and fast-import have nice --import-marks flags which allow
for incremental migrations. However, if there is a mark in
fast-export's file of marks without a corresponding mark in the one for
fast-import, then we run the risk that fast-export tries to send new
objects relative to the mark it knows which fast-import does not,
causing fast-import to fail.
This arises in practice when there is a filter of some sort running
between the fast-export and fast-import processes which prunes some
commits programmatically. Provide such a filter with the ability to
alias pruned commits to their most recent non-pruned ancestor.
Signed-off-by: Elijah Newren
---
Documentation/git-fast-import.txt | 22 +++
fast-import.c | 62 ++-
t/t9300-fast-import.sh| 5 +++
3 files changed, 79 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-fast-import.txt
b/Documentation/git-fast-import.txt
index 4977869465..a3f1e0c5e4 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -337,6 +337,13 @@ and control the current import process. More detailed
discussion
`commit` command. This command is optional and is not
needed to perform an import.
+`alias`::
+ Record that a mark refers to a given object without first
+ creating any new object. Using --import-marks and referring
+ to missing marks will cause fast-import to fail, so aliases
+ can provide a way to set otherwise pruned commits to a valid
+ value (e.g. the nearest non-pruned ancestor).
+
`checkpoint`::
Forces fast-import to close the current packfile, generate its
unique SHA-1 checksum and index, and start a new packfile.
@@ -914,6 +921,21 @@ a data chunk which does not have an LF as its last byte.
+
The `LF` after ` LF` is optional (it used to be required).
+`alias`
+~~~
+Record that a mark refers to a given object without first creating any
+new object.
+
+
+ 'alias' LF
+ mark
+ 'to' SP LF
+ LF?
+
+
+For a detailed description of `` see above under `from`.
+
+
`checkpoint`
Forces fast-import to close the current packfile, start a new one, and to
diff --git a/fast-import.c b/fast-import.c
index 5b9e9e3b02..ac368b3e2b 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2491,18 +2491,14 @@ static void parse_from_existing(struct branch *b)
}
}
-static int parse_from(struct branch *b)
+static int parse_objectish(struct branch *b, const char *objectish)
{
- const char *from;
struct branch *s;
struct object_id oid;
- if (!skip_prefix(command_buf.buf, "from ", &from))
- return 0;
-
oidcpy(&oid, &b->branch_tree.versions[1].oid);
- s = lookup_branch(from);
+ s = lookup_branch(objectish);
if (b == s)
die("Can't create a branch from itself: %s", b->name);
else if (s) {
@@ -2510,8 +2506,8 @@ static int parse_from(struct branch *b)
oidcpy(&b->oid, &s->oid);
oidcpy(&b->branch_tree.versions[0].oid, t);
oidcpy(&b->branch_tree.versions[1].oid, t);
- } else if (*from == ':') {
- uintmax_t idnum = parse_mark_ref_eol(from);
+ } else if (*objectish == ':') {
+ uintmax_t idnum = parse_mark_ref_eol(objectish);
struct object_entry *oe = find_mark(idnum);
if (oe->type != OBJ_COMMIT)
die("Mark :%" PRIuMAX " not a commit", idnum);
@@ -2525,13 +2521,13 @@ static int parse_from(struct branch *b)
} else
parse_from_existing(b);
}
- } else if (!get_oid(from, &b->oid)) {
+ } else if (!get_oid(objectish, &b->oid)) {
parse_from_existing(b);
if (is_null_oid(&b->oid))
b->delete = 1;
}
else
- die("Invalid ref name or SHA1 expression: %s", from);
+ die("Invalid ref name or SHA1 expression: %s", objectish);
if (b->branch_tree.tree && !oideq(&oid,
&b->branch_tree.versions[1].oid)) {
release_tree_content_recursive(b->branch_tree.tree);
@@ -2542,6 +2538,26 @@ static int parse_from(struct branch *b)
return 1;
}
+static int parse_from(struct branch *b)
+{
+ const char *from;
+
+ if (!skip_prefix(command_buf.buf, "from ", &from))
+ return 0;
+
+ return parse_objectish(b, from);
+}
+
+static int parse_objectish_with_prefix(struct branch *b, const char *prefix)
+{
+ const char *base;
+
+ if (!skip_prefix(command_buf.buf, prefix, &base))
+ return 0;
+
+ return parse_objectish(b, base);
+}
+
static struct hash_list *parse_merge(unsigned int *count)
{
struct hash_list *list = NULL, **tail = &list, *n;
@@ -3087,6 +3103,28 @@ static void pa
[PATCH -v3 3/8] fast-import: allow tags to be identified by mark labels
Mark identifiers are used in fast-export and fast-import to provide a label to refer to earlier content. Blobs are given labels because they need to be referenced in the commits where they first appear with a given filename, and commits are given labels because they can be the parents of other commits. Tags were never given labels, probably because they were viewed as unnecessary, but that presents two problems: 1. It leaves us without a way of referring to previous tags if we want to create a tag of a tag (or higher nestings). 2. It leaves us with no way of recording that a tag has already been imported when using --export-marks and --import-marks. Fix these problems by allowing an optional mark label for tags. Signed-off-by: Elijah Newren --- Documentation/git-fast-import.txt | 1 + fast-import.c | 3 ++- t/t9300-fast-import.sh| 19 +++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index 0bb276269e..4977869465 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -774,6 +774,7 @@ lightweight (non-annotated) tags see the `reset` command below. 'tag' SP LF + mark? 'from' SP LF original-oid? 'tagger' (SP )? SP LT GT SP LF diff --git a/fast-import.c b/fast-import.c index caae0819f5..5b9e9e3b02 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2713,6 +2713,7 @@ static void parse_new_tag(const char *arg) first_tag = t; last_tag = t; read_next_command(); + parse_mark(); /* from ... */ if (!skip_prefix(command_buf.buf, "from ", &from)) @@ -2769,7 +2770,7 @@ static void parse_new_tag(const char *arg) strbuf_addbuf(&new_data, &msg); free(tagger); - if (store_object(OBJ_TAG, &new_data, NULL, &t->oid, 0)) + if (store_object(OBJ_TAG, &new_data, NULL, &t->oid, next_mark)) t->pack_id = MAX_PACK_ID; else t->pack_id = pack_id; diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 74bc41333b..3ad2b2f1ba 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -94,6 +94,23 @@ test_expect_success 'A: create pack from stdin' ' reset refs/tags/to-be-deleted from + tag nested + mark :6 + from :4 + data <
[PATCH -v3 6/8] fast-export: allow user to request tags be marked with --mark-tags
Add a new option, --mark-tags, which will output mark identifiers with
each tag object. This improves the incremental export story with
--export-marks since it will allow us to record that annotated tags have
been exported, and it is also needed as a step towards supporting nested
tags.
Signed-off-by: Elijah Newren
---
Documentation/git-fast-export.txt | 17 +
builtin/fast-export.c | 7 +++
t/t9350-fast-export.sh| 14 ++
3 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-fast-export.txt
b/Documentation/git-fast-export.txt
index cc940eb9ad..c522b34f7b 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -75,11 +75,20 @@ produced incorrect results if you gave these options.
Before processing any input, load the marks specified in
. The input file must exist, must be readable, and
must use the same format as produced by --export-marks.
+
+--mark-tags::
+ In addition to labelling blobs and commits with mark ids, also
+ label tags. This is useful in conjunction with
+ `--export-marks` and `--import-marks`, and is also useful (and
+ necessary) for exporting of nested tags. It does not hurt
+ other cases and would be the default, but many fast-import
+ frontends are not prepared to accept tags with mark
+ identifiers.
+
-Any commits that have already been marked will not be exported again.
-If the backend uses a similar --import-marks file, this allows for
-incremental bidirectional exporting of the repository by keeping the
-marks the same across runs.
+Any commits (or tags) that have already been marked will not be
+exported again. If the backend uses a similar --import-marks file,
+this allows for incremental bidirectional exporting of the repository
+by keeping the marks the same across runs.
--fake-missing-tagger::
Some old repositories have tags without a tagger. The
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 575e47833b..d32e1e9327 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -40,6 +40,7 @@ static int no_data;
static int full_tree;
static int reference_excluded_commits;
static int show_original_ids;
+static int mark_tags;
static struct string_list extra_refs = STRING_LIST_INIT_NODUP;
static struct string_list tag_refs = STRING_LIST_INIT_NODUP;
static struct refspec refspecs = REFSPEC_INIT_FETCH;
@@ -861,6 +862,10 @@ static void handle_tag(const char *name, struct tag *tag)
if (starts_with(name, "refs/tags/"))
name += 10;
printf("tag %s\n", name);
+ if (mark_tags) {
+ mark_next_object(&tag->object);
+ printf("mark :%"PRIu32"\n", last_idnum);
+ }
if (tagged_mark)
printf("from :%d\n", tagged_mark);
else
@@ -1165,6 +1170,8 @@ int cmd_fast_export(int argc, const char **argv, const
char *prefix)
&reference_excluded_commits, N_("Reference parents
which are not in fast-export stream by object id")),
OPT_BOOL(0, "show-original-ids", &show_original_ids,
N_("Show original object ids of blobs/commits")),
+ OPT_BOOL(0, "mark-tags", &mark_tags,
+ N_("Label tags with mark ids")),
OPT_END()
};
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index ea84e2f173..b3fca6ffba 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -66,6 +66,20 @@ test_expect_success 'fast-export ^muss^{commit} muss' '
test_cmp expected actual
'
+test_expect_success 'fast-export --mark-tags ^muss^{commit} muss' '
+ git fast-export --mark-tags --tag-of-filtered-object=rewrite
^muss^{commit} muss >actual &&
+ cat >expected <<-EOF &&
+ tag muss
+ mark :1
+ from $(git rev-parse --verify muss^{commit})
+ $(git cat-file tag muss | grep tagger)
+ data 9
+ valentin
+
+ EOF
+ test_cmp expected actual
+'
+
test_expect_success 'fast-export master~2..master' '
git fast-export master~2..master >actual &&
--
2.23.0.264.g3b9f7f2fc6
[PATCH -v3 7/8] t9350: add tests for tags of things other than a commit
Multiple changes here: * add a test for a tag of a blob * add a test for a tag of a tag of a commit * add a comment to the tests for (possibly nested) tags of trees, making it clear that these tests are doing much less than you might expect Signed-off-by: Elijah Newren --- t/t9350-fast-export.sh | 31 +++ 1 file changed, 31 insertions(+) diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh index b3fca6ffba..9ab281e4b9 100755 --- a/t/t9350-fast-export.sh +++ b/t/t9350-fast-export.sh @@ -540,10 +540,41 @@ test_expect_success 'tree_tag'' ' # NEEDSWORK: not just check return status, but validate the output +# Note that these tests DO NOTHING other than print a warning that +# they are ommitting the one tag we asked them to export (because the +# tags resolve to a tree). They exist just to make sure we do not +# abort but instead just warn. test_expect_success 'tree_tag-obj''git fast-export tree_tag-obj' test_expect_success 'tag-obj_tag' 'git fast-export tag-obj_tag' test_expect_success 'tag-obj_tag-obj' 'git fast-export tag-obj_tag-obj' +test_expect_success 'handling tags of blobs' ' + git tag -a -m "Tag of a blob" blobtag $(git rev-parse master:file) && + git fast-export blobtag >actual && + cat >expect <<-EOF && + blob + mark :1 + data 9 + die Luft + + tag blobtag + from :1 + tagger $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data 14 + Tag of a blob + + EOF + test_cmp expect actual +' + +test_expect_failure 'handling nested tags' ' + git tag -a -m "This is a nested tag" nested muss && + git fast-export --mark-tags nested >output && + grep "^from $ZERO_OID$" output && + grep "^tag nested$" output >tag_lines && + test_line_count = 2 tag_lines +' + test_expect_success 'directory becomes symlink'' git init dirtosymlink && git init result && -- 2.23.0.264.g3b9f7f2fc6
[PATCH -v3 8/8] fast-export: handle nested tags
Signed-off-by: Elijah Newren
---
builtin/fast-export.c | 30 ++
t/t9350-fast-export.sh | 2 +-
2 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d32e1e9327..58a74de42a 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -843,22 +843,28 @@ static void handle_tag(const char *name, struct tag *tag)
free(buf);
return;
case REWRITE:
- if (tagged->type != OBJ_COMMIT) {
- die("tag %s tags unexported %s!",
- oid_to_hex(&tag->object.oid),
- type_name(tagged->type));
- }
- p = rewrite_commit((struct commit *)tagged);
- if (!p) {
- printf("reset %s\nfrom %s\n\n",
- name, oid_to_hex(&null_oid));
- free(buf);
- return;
+ if (tagged->type == OBJ_TAG && !mark_tags) {
+ die(_("Error: Cannot export nested tags unless
--mark-tags is specified."));
+ } else if (tagged->type == OBJ_COMMIT) {
+ p = rewrite_commit((struct commit *)tagged);
+ if (!p) {
+ printf("reset %s\nfrom %s\n\n",
+ name, oid_to_hex(&null_oid));
+ free(buf);
+ return;
+ }
+ tagged_mark = get_object_mark(&p->object);
+ } else {
+ /* tagged->type is either OBJ_BLOB or OBJ_TAG */
+ tagged_mark = get_object_mark(tagged);
}
- tagged_mark = get_object_mark(&p->object);
}
}
+ if (tagged->type == OBJ_TAG) {
+ printf("reset %s\nfrom %s\n\n",
+ name, oid_to_hex(&null_oid));
+ }
if (starts_with(name, "refs/tags/"))
name += 10;
printf("tag %s\n", name);
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 9ab281e4b9..2e4e214815 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -567,7 +567,7 @@ test_expect_success 'handling tags of blobs' '
test_cmp expect actual
'
-test_expect_failure 'handling nested tags' '
+test_expect_success 'handling nested tags' '
git tag -a -m "This is a nested tag" nested muss &&
git fast-export --mark-tags nested >output &&
grep "^from $ZERO_OID$" output &&
--
2.23.0.264.g3b9f7f2fc6
[PATCH -v3 5/8] fast-export: add support for --import-marks-if-exists
fast-import has support for both an --import-marks flag and an
--import-marks-if-exists flag; the latter of which will not die() if the
file does not exist. fast-export only had support for an --import-marks
flag; add an --import-marks-if-exists flag for consistency.
Signed-off-by: Elijah Newren
---
builtin/fast-export.c | 23 +++
t/t9350-fast-export.sh | 10 --
2 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 5822271c6b..575e47833b 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -1052,11 +1052,16 @@ static void export_marks(char *file)
error("Unable to write marks file %s.", file);
}
-static void import_marks(char *input_file)
+static void import_marks(char *input_file, int check_exists)
{
char line[512];
- FILE *f = xfopen(input_file, "r");
+ FILE *f;
+ struct stat sb;
+
+ if (check_exists && stat(input_file, &sb))
+ return;
+ f = xfopen(input_file, "r");
while (fgets(line, sizeof(line), f)) {
uint32_t mark;
char *line_end, *mark_end;
@@ -1120,7 +1125,9 @@ int cmd_fast_export(int argc, const char **argv, const
char *prefix)
struct rev_info revs;
struct object_array commits = OBJECT_ARRAY_INIT;
struct commit *commit;
- char *export_filename = NULL, *import_filename = NULL;
+ char *export_filename = NULL,
+*import_filename = NULL,
+*import_filename_if_exists = NULL;
uint32_t lastimportid;
struct string_list refspecs_list = STRING_LIST_INIT_NODUP;
struct string_list paths_of_changed_objects = STRING_LIST_INIT_DUP;
@@ -1140,6 +1147,10 @@ int cmd_fast_export(int argc, const char **argv, const
char *prefix)
N_("Dump marks to this file")),
OPT_STRING(0, "import-marks", &import_filename, N_("file"),
N_("Import marks from this file")),
+ OPT_STRING(0, "import-marks-if-exists",
+&import_filename_if_exists,
+N_("file"),
+N_("Import marks from this file if it exists")),
OPT_BOOL(0, "fake-missing-tagger", &fake_missing_tagger,
N_("Fake a tagger when tags lack one")),
OPT_BOOL(0, "full-tree", &full_tree,
@@ -1187,8 +1198,12 @@ int cmd_fast_export(int argc, const char **argv, const
char *prefix)
if (use_done_feature)
printf("feature done\n");
+ if (import_filename && import_filename_if_exists)
+ die(_("Cannot pass both --import-marks and
--import-marks-if-exists"));
if (import_filename)
- import_marks(import_filename);
+ import_marks(import_filename, 0);
+ else if (import_filename_if_exists)
+ import_marks(import_filename_if_exists, 1);
lastimportid = last_idnum;
if (import_filename && revs.prune_data.nr)
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index d32ff41859..ea84e2f173 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -580,17 +580,15 @@ test_expect_success 'fast-export quotes pathnames' '
'
test_expect_success 'test bidirectionality' '
- >marks-cur &&
- >marks-new &&
git init marks-test &&
- git fast-export --export-marks=marks-cur --import-marks=marks-cur
--branches | \
- git --git-dir=marks-test/.git fast-import --export-marks=marks-new
--import-marks=marks-new &&
+ git fast-export --export-marks=marks-cur
--import-marks-if-exists=marks-cur --branches | \
+ git --git-dir=marks-test/.git fast-import --export-marks=marks-new
--import-marks-if-exists=marks-new &&
(cd marks-test &&
git reset --hard &&
echo Wohlauf > file &&
git commit -a -m "back in time") &&
- git --git-dir=marks-test/.git fast-export --export-marks=marks-new
--import-marks=marks-new --branches | \
- git fast-import --export-marks=marks-cur --import-marks=marks-cur
+ git --git-dir=marks-test/.git fast-export --export-marks=marks-new
--import-marks-if-exists=marks-new --branches | \
+ git fast-import --export-marks=marks-cur
--import-marks-if-exists=marks-cur
'
cat > expected << EOF
--
2.23.0.264.g3b9f7f2fc6
Re: What's cooking in git.git (Oct 2019, #01; Thu, 3)
On Wed, Oct 2, 2019 at 10:22 PM Junio C Hamano wrote: > > * en/fast-imexport-nested-tags (2019-10-02) 8 commits > - fast-export: handle nested tags > - t9350: add tests for tags of things other than a commit > - fast-export: allow user to request tags be marked with --mark-tags > - fast-export: add support for --import-marks-if-exists > - fast-import: add support for new 'alias' command > - fast-import: allow tags to be identified by mark labels > - fast-import: fix handling of deleted tags > - fast-export: fix exporting a tag and nothing else > > Updates to fast-import/export. > > Will merge to 'next'. Actually, René posted a code cleanup suggestion for patch 2/8, so I sent a V3 re-roll[1]. Could you pick up V3 instead of merging V2 down to next? [1] https://public-inbox.org/git/20191003202709.26279-1-new...@gmail.com/
Re: [Outreachy] Outreachy internship program
Hi George, it sounds like you are probably using Git for Windows (https://github.com/git-for-windows/git). I'm actually not very familiar with how folks who primarily use GfW as their client manage their contributions to the main Git project. However, I know there are plenty - the GfW maintainer is an active contributor upstream. I'm CCing the Git mailing list as well as the GfW maintainer in the hopes that you can get some help from somebody who regularly uses the workflow you're trying to achieve. :) Unfortunately I use Linux everywhere and so I can't try to replicate what you're doing - but once you have a good workflow and are able to finish the My First Contribution tutorial we should still be able to work together. Can you be specific about which "official website" you downloaded Git from (share a URL), and paste the command you run and error message you receive? Please also share the output you see when you run "uname -a" in Git Bash. - Emily On Thu, Oct 3, 2019 at 12:40 PM gespinoz gespinoz wrote: > > Hello, > > Great! Thanks for the tips Emily! This mentor and internship program sounds > awesome which is why I decided to apply. I’ll look through it to see if > there’s a specific micro project available later on today and run it through > you. > > I am having one slight issue, I’m not sure why I can’t use the “make” command > to run Makefile, I’ve done it at school in the past so I was a bit familiar > with Makefile and how it helps compile things. This is what I did. I > downloaded git from the official git website, installed it, then I found the > repository link and I cloned it in the git bash client to my desktop. I’m > using windows at home but at school I used an iMac and when I cloned > repositories I used iTerm and pushed to git within iTerm. I’m guessing the > git bash is similar since I was able to open and edit files using vim > commands similar in iTerm when I was going through the walkthrough. So now > I’m stuck on the “make” step to see if I added the psuh feature in correctly. > I also noticed when I looked at the INSTALL page on github it made it seem > like maybe installing it through the website wasn’t the right idea? Maybe > that’s why I can’t use make? I also can’t use man correctly. For both I get > bash command does not exist. Should I install something similar to iTerm > instead on windows and just start fresh? ty. > > George! > > On Thu, Oct 3, 2019 at 11:14 AM Emily Shaffer wrote: >> >> Hi George, >> >> Great to hear that you're walking through the MyFirstContribution >> tutorial - that's a great introduction to how the Git project gets >> stuff done. >> >> This is my first time mentoring for Outreachy, but as I understand it, >> you will start by doing a microproject so everybody can see if you are >> a good fit for the project. The community discussed the list of >> microprojects for applicants to try in this mailing list thread: >> https://public-inbox.org/git/20190916184208.gb17...@google.com/ >> I don't think you need to wade through the replies on that thread to >> determine whether the microproject you are interested in is available, >> although you're certainly welcome to. You can also ask me if you see a >> microproject you are interested in and I will be happy to help >> summarize it and point you in the right direction :) >> >> It is also probably a good idea for you to search that mailing list >> archive for "[Outreachy]" so you can see what Outreachy interns have >> done in the past and try to emulate how they submitted finished >> microprojects. >> >> The tutorial you said you're reading covers how to send your >> contributions for review when you're done, but if you find you're >> having trouble or want someone to check that you've formatted it >> right, you can let me know! >> >> - Emily >> >> On Wed, Oct 2, 2019 at 9:55 PM gespinoz gespinoz >> wrote: >> > >> > Hello Emily, >> > >> > How are you? My name is George Espinoza and I am one of the applicants in >> > the Outreachy internship program. I hope you are doing well! I wanted to >> > introduce myself and connect as I am interested in the Git open source >> > project that you are mentoring. I'm currently creating my work environment >> > and tinkering with the git program while doing the myfirstcontribution >> > walk-through. I have had a bit of experience using git for a school i >> > attended over the summer that used it for clone and pushing repositories. >> > We worked with iTerm and I learned how to use vim as well. I know some >> > basics, in no way a master yet/ I hope to learn more as I progress in >> > making contributions! >> > >> > After I set up my environment I will join the IRC channel and introduce >> > myself in the project's public chat and go from there searching how and >> > what I should first contribute. I'll also join the mailing list. If you >> > have any tips or advice that would be great! I'm looking forward to >> > working with you and
[PATCH 0/1] gitk: Addressing error running on MacOS with large repos.
gitk: no need to specify all refs, since git log --all is the same as list is all the refs/commit ids. Also Mac OS has a limit on size of the list of params for a command line Arash Bannazadeh-Mahani (1): gitk: Addressing error running on MacOS with large repos. gitk-git/gitk | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) base-commit: bc12974a897308fd3254cf0cc90319078fe45eea Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-375%2Faxbannaz%2Fuse.all-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-375/axbannaz/use.all-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/375 -- gitgitgadget
[PATCH 1/1] gitk: Addressing error running on MacOS with large repos.
From: Arash Bannazadeh-Mahani
The change is stemmed from a problem on the MacOS where, if --all
is passed to gitk it should show all the refs/commits graphically.
However, on really large git repos, in my instance a git repo with
over 52,000 commits, gitk will report an error,
"Error executing git log: couldn't execute "git": argument list too long".
Mac OS has a limit of which my large repo exceeds. This works fine on Linux,
however, not sure about Windows.
Looking at gitk script, the decision to have all commit-ids on the command line
comes from return value of parseviewargs() function which uses the value of
"allknown" to return. If it is '1' then --all is translated to a string of all
the commit-ids in the repo, otherwise --all is passed as-is to `git log` cli,
which according to git-log man page it is the same as listing all the
commit-ids.
So, this change is to prevent --all option from being expanded into list
of all refs on the command line.
Signed-off-by: Arash Bannazadeh-Mahani
---
gitk-git/gitk | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index abe4805ade..96634d9d33 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -250,8 +250,13 @@ proc parseviewargs {n arglist} {
set nextisval 1
lappend glflags $arg
}
- "--not" - "--all" {
+ "--not" {
lappend revargs $arg
+ }
+ "--all" {
+ # we recognize this argument;
+ # no expansion needed, use it with 'git log' as-is
+ set allknown 0
}
"--merge" {
set vmergeonly($n) 1
--
gitgitgadget
Re: [PATCH 2/2] git-gui: support for diff3 conflict style
On 03/10/19 09:02PM, Philip Oakley wrote: > On 30/09/2019 13:17, Bert Wesarg wrote: > > Pratyush, > > > > On Sun, Sep 29, 2019 at 5:04 PM Pratyush Yadav > > wrote: > > > Hi Philip, Bert, > > > > > > Is there any way I can test this change? Philip, I ran the rebase you > > > mention in the GitHub issue [0], and I get that '9c8cba6862abe5ac821' is > > > an unknown revision. > > > > > > Is there any quick way I can reproduce this (maybe on a sample repo)? > > The easiest way to produce a merge conflict, is to change the same > > line differently in two branches and try to merge them. I added a > > fast-import file to demonstrate this for you. > > > > $ git init foo > > $ cd foo > > $ git fast-import <../conflict-merge.fi > > $ git reset --hard master > > $ git merge branch > > > > this gets you into the conflict, probably the usual style. Which looks > > in liek this on the terminal: > > > > @@@ -2,7 -2,7 +2,11 @@@ Lorem ipsum dolor sit amet, consectetu > >Sed feugiat nisl eget efficitur ultrices. > >Nunc cursus metus rutrum, mollis lorem vitae, hendrerit mi. > >Aenean vestibulum ante ac libero venenatis, non hendrerit orci pharetra. > > ++<<< HEAD > > +Proin bibendum purus ut est tristique, non pharetra dui consectetur. > > ++=== > > + Proin placerat leo malesuada lacinia lobortis. > > ++>>> branch > >Pellentesque aliquam libero et nisi scelerisque commodo. > >Quisque id velit sed magna molestie porttitor. > >Vivamus sed urna in risus molestie ultricies. > > > > and this in git gui: https://kgab.selfhost.eu/s/gHHaQqowGp7mXEb > > The snapshot of the Gui looks just the thing! (I've been away). > > I'm sure this would solve my immediate issue. > > My only remaining bikeshed question it prompted was to check which parts > would be committed as part of committing the whole "hunk". But haven't had > time to look at all! I'm not sure what you mean by "committing the whole hunk". In a merge conflict state, you don't get the usual "Stage hunk" and "Stage lines" options, but instead get 3 options: Use Remote Version Use Local Version Revert To Base You can use these to choose how you want to resolve the conflict. These 3 options seem to work fine on my quick testing. -- Regards, Pratyush Yadav
Re: [PATCH 0/1] fsmonitor: don't fill bitmap with entries to be removed
Hi all, On Thu, 3 Oct 2019, William Baker via GitGitGadget wrote: > This patch series fixes a segfault that I encountered while testing > fsmonitor. Under some circumstances, the fsmonitor extension was being > written with too many bits, and subsequent git commands would segfault when > trying to apply the bits to the index. > > As part of these changes I've added some BUG checks that would have helped > catch this problem sooner. Special thanks to Dscho for pointing me in the > right direction and suggesting a test that can reproduce the issue. > > Thanks, William Please note that I was involved with the development of this patch, reviewed a couple of iterations internally and am implictly okay with this first public iteration. Ciao, Dscho > > William Baker (1): > fsmonitor: don't fill bitmap with entries to be removed > > fsmonitor.c | 29 - > t/t7519-status-fsmonitor.sh | 12 > t/t7519/fsmonitor-env | 24 > 3 files changed, 60 insertions(+), 5 deletions(-) > create mode 100755 t/t7519/fsmonitor-env > > > base-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9 > Published-As: > https://github.com/gitgitgadget/git/releases/tag/pr-372%2Fwilbaker%2Ffix_git_fsmonitor_crash-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git > pr-372/wilbaker/fix_git_fsmonitor_crash-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/372 > -- > gitgitgadget >
Re: [Outreachy] Outreachy internship program
Hi, yes, there is no `make` or `gcc` available on Windows by default. You will have to download and install the Git for Windows SDK: https://gitforwindows.org/#download-sdk Ciao, Johannes On Thu, 3 Oct 2019, Emily Shaffer wrote: > Hi George, it sounds like you are probably using Git for Windows > (https://github.com/git-for-windows/git). > > I'm actually not very familiar with how folks who primarily use GfW as > their client manage their contributions to the main Git project. > However, I know there are plenty - the GfW maintainer is an active > contributor upstream. > > I'm CCing the Git mailing list as well as the GfW maintainer in the > hopes that you can get some help from somebody who regularly uses the > workflow you're trying to achieve. :) > > Unfortunately I use Linux everywhere and so I can't try to replicate > what you're doing - but once you have a good workflow and are able to > finish the My First Contribution tutorial we should still be able to > work together. > > Can you be specific about which "official website" you downloaded Git > from (share a URL), and paste the command you run and error message > you receive? Please also share the output you see when you run "uname > -a" in Git Bash. > > - Emily > > > On Thu, Oct 3, 2019 at 12:40 PM gespinoz gespinoz > wrote: > > > > Hello, > > > > Great! Thanks for the tips Emily! This mentor and internship program sounds > > awesome which is why I decided to apply. I’ll look through it to see if > > there’s a specific micro project available later on today and run it > > through you. > > > > I am having one slight issue, I’m not sure why I can’t use the “make” > > command to run Makefile, I’ve done it at school in the past so I was a bit > > familiar with Makefile and how it helps compile things. This is what I did. > > I downloaded git from the official git website, installed it, then I found > > the repository link and I cloned it in the git bash client to my desktop. > > I’m using windows at home but at school I used an iMac and when I cloned > > repositories I used iTerm and pushed to git within iTerm. I’m guessing the > > git bash is similar since I was able to open and edit files using vim > > commands similar in iTerm when I was going through the walkthrough. So now > > I’m stuck on the “make” step to see if I added the psuh feature in > > correctly. I also noticed when I looked at the INSTALL page on github it > > made it seem like maybe installing it through the website wasn’t the right > > idea? Maybe that’s why I can’t use make? I also can’t use man correctly. > > For both I get bash command does not exist. Should I install something > > similar to iTerm instead on windows and just start fresh? ty. > > > > George! > > > > On Thu, Oct 3, 2019 at 11:14 AM Emily Shaffer > > wrote: > >> > >> Hi George, > >> > >> Great to hear that you're walking through the MyFirstContribution > >> tutorial - that's a great introduction to how the Git project gets > >> stuff done. > >> > >> This is my first time mentoring for Outreachy, but as I understand it, > >> you will start by doing a microproject so everybody can see if you are > >> a good fit for the project. The community discussed the list of > >> microprojects for applicants to try in this mailing list thread: > >> https://public-inbox.org/git/20190916184208.gb17...@google.com/ > >> I don't think you need to wade through the replies on that thread to > >> determine whether the microproject you are interested in is available, > >> although you're certainly welcome to. You can also ask me if you see a > >> microproject you are interested in and I will be happy to help > >> summarize it and point you in the right direction :) > >> > >> It is also probably a good idea for you to search that mailing list > >> archive for "[Outreachy]" so you can see what Outreachy interns have > >> done in the past and try to emulate how they submitted finished > >> microprojects. > >> > >> The tutorial you said you're reading covers how to send your > >> contributions for review when you're done, but if you find you're > >> having trouble or want someone to check that you've formatted it > >> right, you can let me know! > >> > >> - Emily > >> > >> On Wed, Oct 2, 2019 at 9:55 PM gespinoz gespinoz > >> wrote: > >> > > >> > Hello Emily, > >> > > >> > How are you? My name is George Espinoza and I am one of the applicants > >> > in the Outreachy internship program. I hope you are doing well! I wanted > >> > to introduce myself and connect as I am interested in the Git open > >> > source project that you are mentoring. I'm currently creating my work > >> > environment and tinkering with the git program while doing the > >> > myfirstcontribution walk-through. I have had a bit of experience using > >> > git for a school i attended over the summer that used it for clone and > >> > pushing repositories. We worked with iTerm and I learned how to use vim > >> > as well. I know some basics, in
Re: [PATCH 2/2] git-gui: support for diff3 conflict style
On 03/10/2019 21:54, Pratyush Yadav wrote: My only remaining bikeshed question it prompted was to check which parts would be committed as part of committing the whole "hunk". But haven't had time to look at all! I'm not sure what you mean by "committing the whole hunk". In a merge conflict state, you don't get the usual "Stage hunk" and "Stage lines" options, but instead get 3 options: Use Remote Version Use Local Version Revert To Base You can use these to choose how you want to resolve the conflict. These 3 options seem to work fine on my quick testing. That looks like just the answer I was hoping for! Thanks. -- Philip
Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts
Hi Harish, Thanks for the patch. Unfortunately, it seems your mail client messed up the formatting, and the patch won't apply. I'm guessing it is because your mail client broke long lines into two, messing up the diff. We use an email-based workflow, so please either configure your mail client so it doesn't munge patches, or use `git send-email`. You can find a pretty good tutorial on sending patches via email over at [0]. The tutorial is for git.git, but works for git-gui.git as well. If you feel more comfortable with GitHub pull requests, please take a look at Gitgitgadget [1]. Johannes (in Cc) has used it recently to send patches based on the git-gui repo (AFAIK, it was originally designed only for the git.git repo). Maybe ask the folks over there how they do it. One more thing: your patch is based on the main Git repo. That repo is not where git-gui development takes place. The current "official" repo for git-gui is over at [2]. Please base your patches on top of that repo. [0] https://matheustavares.gitlab.io/posts/first-steps-contributing-to-git#submitting-patches [1] https://gitgitgadget.github.io/ [2] https://github.com/prati0100/git-gui Now on to the nitty gritty details. I like the idea. In fact, there were some discussions recently about having configurable key bindings for _all_ shortcuts in git-gui. Nothing concrete has been done in that direction yet though. But I feel like this is a pretty good first step. On 03/10/19 08:18PM, harish k wrote: > Hi All, > I', Just reopening this feature request. > A quick summary of my proposal is given below. > > 1. This PR will allow an additional configuration option > "guitool..gitgui-shortcut" which will allow us to specify > keyboard shortcut for custom commands in git-gui A pretty nice way of doing it. But I would _really_ like it if there was an option in the "create tool" dialog to specify the shortcut. People of a gui tool shouldn't have to mess around with config files as much as possible. > 2. Even there exists a parameter called "guitool..shortcut" > which is used by git-cola, I suggest to keep this new additional > config parameter as an independent config parameter, which will not > interfere with git-cola in any way, because, both are different > applications and it may have different "built-in" shortcuts already > assigned. So, sharing shortcut scheme between two apps is not a good > idea. David has advocated inter-operability between git-gui and git-cola. While I personally don't know how many people actually use both the tools at the same time, it doesn't sound like a bad idea either. So, sharing shortcuts with git-cola would be nice. Of course, it would then mean that we would have to parse the config parameter before feeding them to `bind`. I don't suppose that should be something too complicated to do, but I admit I haven't looked too deeply into it. I'd like to hear what other people think about whether it is worth the effort to inter-operate with git-cola. > 3. New parameter will expect shortcut combinations specified in TCL/TK > 's format and we will not be doing any processing on it. Will keep it > simple. Are you sure that is a good idea? I think we should at least make sure we are not binding some illegal sequence, and if we are, we should warn the user about it. And a much more important case would be when a user over-writes a pre-existing shortcut for other commands like "commit", "reset", etc. In that case, the menu entires of those commands would still be labelled with the shortcut, but it won't actually work. Yes, your current implementation keeps things simple, but I think some light processing would be beneficial. And if we do decide to go the inter-operability with git-cola route, then processing would be needed anyway, and we can validate there. > --- > Documentation/config/guitool.txt | 15 +++ > git-gui/lib/tools.tcl| 15 --- > 2 files changed, 27 insertions(+), 3 deletions(-) > > diff --git a/Documentation/config/guitool.txt > b/Documentation/config/guitool.txt > index 43fb9466ff..79dac23ca3 100644 > --- a/Documentation/config/guitool.txt > +++ b/Documentation/config/guitool.txt > @@ -48,3 +48,18 @@ guitool..prompt:: > Specifies the general prompt string to display at the top of > the dialog, before subsections for 'argPrompt' and 'revPrompt'. > The default value includes the actual command. > + > +guitool..gitgui-shortcut > + Specifies a keyboard shortcut for the custom tool in the git-gui > + application. The value must be a valid string ( without "<" , ">" wrapper ) > + understood by the TCL/TK 's bind command.See > https://www.tcl.tk/man/tcl8.4/TkCmd/bind.htm > + for more details about the supported values. Avoid creating shortcuts that > + conflict with existing built-in `git gui` shortcuts. > + Example: > + [guitool "Terminal"] > + cmd = gnome-terminal -e zsh > + noconsole = yes > + gitgui-shortcut = "Control-y" > +
Re: [Outreachy] Outreachy internship program
On 03/10/2019 21:35, Emily Shaffer wrote: Hi George, it sounds like you are probably using Git for Windows (https://github.com/git-for-windows/git). I'm actually not very familiar with how folks who primarily use GfW as their client manage their contributions to the main Git project. However, I know there are plenty - the GfW maintainer is an active contributor upstream. I'm CCing the Git mailing list as well as the GfW maintainer in the hopes that you can get some help from somebody who regularly uses the workflow you're trying to achieve. :) Unfortunately I use Linux everywhere and so I can't try to replicate what you're doing - but once you have a good workflow and are able to finish the My First Contribution tutorial we should still be able to work together. Can you be specific about which "official website" you downloaded Git from (share a URL), and paste the command you run and error message you receive? Please also share the output you see when you run "uname -a" in Git Bash. Hi, The top level domain would be https://gitforwindows.org/ The download button should give the same pure run-time as the git-scm website. However for this case you should go via the "Contribute" button to get the full Windows SDK that will provide all those missing items for a reasonable compile experience (including 'make' !). The SDK and the regular 'Program Files' Git are independent so can both be used. I tend to develop on to of the patched Windows version of git, and usually there is enough separation that the patches transfer direct to the Linux upstream. HTHs Philip - Emily On Thu, Oct 3, 2019 at 12:40 PM gespinoz gespinoz wrote: Hello, Great! Thanks for the tips Emily! This mentor and internship program sounds awesome which is why I decided to apply. I’ll look through it to see if there’s a specific micro project available later on today and run it through you. I am having one slight issue, I’m not sure why I can’t use the “make” command to run Makefile, I’ve done it at school in the past so I was a bit familiar with Makefile and how it helps compile things. This is what I did. I downloaded git from the official git website, installed it, then I found the repository link and I cloned it in the git bash client to my desktop. I’m using windows at home but at school I used an iMac and when I cloned repositories I used iTerm and pushed to git within iTerm. I’m guessing the git bash is similar since I was able to open and edit files using vim commands similar in iTerm when I was going through the walkthrough. So now I’m stuck on the “make” step to see if I added the psuh feature in correctly. I also noticed when I looked at the INSTALL page on github it made it seem like maybe installing it through the website wasn’t the right idea? Maybe that’s why I can’t use make? I also can’t use man correctly. For both I get bash command does not exist. Should I install something similar to iTerm instead on windows and just start fresh? ty. George! On Thu, Oct 3, 2019 at 11:14 AM Emily Shaffer wrote: Hi George, Great to hear that you're walking through the MyFirstContribution tutorial - that's a great introduction to how the Git project gets stuff done. This is my first time mentoring for Outreachy, but as I understand it, you will start by doing a microproject so everybody can see if you are a good fit for the project. The community discussed the list of microprojects for applicants to try in this mailing list thread: https://public-inbox.org/git/20190916184208.gb17...@google.com/ I don't think you need to wade through the replies on that thread to determine whether the microproject you are interested in is available, although you're certainly welcome to. You can also ask me if you see a microproject you are interested in and I will be happy to help summarize it and point you in the right direction :) It is also probably a good idea for you to search that mailing list archive for "[Outreachy]" so you can see what Outreachy interns have done in the past and try to emulate how they submitted finished microprojects. The tutorial you said you're reading covers how to send your contributions for review when you're done, but if you find you're having trouble or want someone to check that you've formatted it right, you can let me know! - Emily On Wed, Oct 2, 2019 at 9:55 PM gespinoz gespinoz wrote: Hello Emily, How are you? My name is George Espinoza and I am one of the applicants in the Outreachy internship program. I hope you are doing well! I wanted to introduce myself and connect as I am interested in the Git open source project that you are mentoring. I'm currently creating my work environment and tinkering with the git program while doing the myfirstcontribution walk-through. I have had a bit of experience using git for a school i attended over the summer that used it for clone and pushing repositories. We worked with iTerm and I learned how to use vim as well. I kn
Re: [BUG/PATCH 0/5] t4214: cleanup and demonstrate graph bug
Jeff King writes: > That works for the diagram in the code: > >| *---. >| |\ \ \ >|/ / / / >x 0 1 2 > > where one of the parent lines is collapsing back to the left. But not > for this more mundane case: > > | *-. commit 211232bae64bcc60bbf5d1b5e5b2344c22ed767e > | |\ \ \ \ Merge: fc54a9c30c 9e30dd7c0e c4b83e618f 660265909f b28858bf65 > | | | | | | > > where we go straight down. I'm not sure I've fully grasped it, but it > feels like that distinction is the source of the off-by-one. I'm not > sure how to tell the difference here, though. I think it relies on the > next commit on the left-hand line being the same as the first parent (or > maybe any parent?). > > If I remove the use of parent_in_old_cols entirely, the merge looks > right, but the "collapsing" one is broken (and t4214 fails). > > By the way, a useful trick I stumbled on to look at the coloring across > many such merges: > > git log --graph --format=%h --color | grep -A2 -e - | less -S > > It looks like every octopus in git.git is colored wrong (because they're > the non-collapsing type). Thanks for analysing further. I wonder if new tests added by Denton's BUG/PATCH series cover both kinds? It would be good to make sure that any "fix" corrects known-broken cases while keeping the working cases still working. Thanks.
Re: [PATCH 1/1] stash apply: report status correctly even in a worktree's subdirectory
"Johannes Schindelin via GitGitGadget" writes: > diff --git a/builtin/stash.c b/builtin/stash.c > index b5a301f24d..a1e2e7ae7e 100644 > --- a/builtin/stash.c > +++ b/builtin/stash.c > @@ -497,6 +497,8 @@ static int do_apply_stash(const char *prefix, struct > stash_info *info, >*/ > cp.git_cmd = 1; > cp.dir = prefix; > + argv_array_pushf(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT"=%s", > + absolute_path(get_git_work_tree())); > argv_array_push(&cp.args, "status"); > run_command(&cp); > } Nicely spotted. Exporting GIT_WORK_TREE alone without GIT_DIR feels a bit disturbing, at least to me, though. I wondered if this misbehaves when the end user has GIT_WORK_TREE environment exported, but in such a case, get_git_work_tree() would return that directory, and by re-exporting it to the child process, we would honor the end user's intention, so all is good, I think. Thanks. > diff --git a/t/t3908-stash-in-worktree.sh b/t/t3908-stash-in-worktree.sh > new file mode 100755 > index 00..2b2b366ef9 > --- /dev/null > +++ b/t/t3908-stash-in-worktree.sh > @@ -0,0 +1,27 @@ > +#!/bin/sh > +# > +# Copyright (c) 2019 Johannes E Schindelin > +# > + > +test_description='Test git stash in a worktree' > + > +. ./test-lib.sh > + > +test_expect_success 'setup' ' > + test_commit initial && > + git worktree add wt && > + test_commit -C wt in-worktree > +' > + > +test_expect_success 'apply in subdirectory' ' > + mkdir wt/subdir && > + ( > + cd wt/subdir && > + echo modified >../initial.t && > + git stash && > + git stash apply >out > + ) && > + grep "\.\.\/initial\.t" wt/subdir/out > +' > + > +test_done
Re: [PATCH v2 00/11] New sparse-checkout builtin and "cone" mode
Derrick Stolee writes: > On 9/19/2019 10:43 AM, Derrick Stolee via GitGitGadget wrote: >> This series makes the sparse-checkout feature more user-friendly. While >> there, I also present a way to use a limited set of patterns to gain a >> significant performance boost in very large repositories. >> >> Sparse-checkout is only documented as a subsection of the read-tree docs >> [1], which makes the feature hard to discover. Users have trouble navigating >> the feature, especially at clone time [2], and have even resorted to >> creating their own helper tools [3]. >> >> This series attempts to solve these problems using a new builtin. > > I haven't heard anything about this series since Elijah's careful > review of the RFC. There are definitely areas where this can be > made more robust, but I'd like to save those for a follow-up series. > > Junio: I know you didn't track this in the recent "what's cooking" > list, and I don't expect you to take it until I re-roll v3 to > include the .gitignore interaction I already pointed out. I have made a mental note that says "expecting v3, a reroll. cf. <7d87fe4b-160c-34c2-db6d-4a56fd919...@gmail.com>"; there is no existing entry to hang it below in the "what's cooking" report, though X-<.
Re: What's cooking in git.git (Oct 2019, #01; Thu, 3)
Emily Shaffer writes: > On Thu, Oct 03, 2019 at 02:04:39PM +0900, Junio C Hamano wrote: >> Here are the topics that have been cooking. Commits prefixed with >> '-' are only in 'pu' (proposed updates) while commits prefixed with >> '+' are in 'next'. The ones marked with '.' do not appear in any of >> the integration branches, but I am still holding onto them. >> >> You can find the changes described here in the integration branches >> of the repositories listed at >> >> http://git-blame.blogspot.com/p/git-public-repositories.html >> > > Hi Junio, > > How do you feel about > https://public-inbox.org/git/20190930220355.108394-1-emilyshaf...@google.com/ > which fixes a user-facing SIGABRT during partial clone operations? It's > reviewed by Peff and Christian. Already picked up on the tip of cc/multi-promisor and pushed out, I think, as part of yesterday's integration cycle. Thanks for fixing.
Re: What's cooking in git.git (Oct 2019, #01; Thu, 3)
Elijah Newren writes: > Actually, René posted a code cleanup suggestion for patch 2/8, so I > sent a V3 re-roll[1]. Could you pick up V3 instead of merging V2 down > to next? > > [1] https://public-inbox.org/git/20191003202709.26279-1-new...@gmail.com/ Thanks for stopping me. Will take a look.
bad error message - Not a git repository (or any of the parent directories): .git
when running git commands outside of a git repo, we often see: fatal: Not a git repository (or any of the parent directories): .git such a lame message lol. can we get an absolute path on this message in future git versions, eg: Not a git repository (or any of the parent directories): /home/ubuntu/xyz/.git ty -alex -- Alexander D. Mills New cell phone # (415)730-1805 alexander.d.mi...@gmail.com www.linkedin.com/pub/alexander-mills/b/7a5/418/
Re: [PATCH v2 01/13] push: do not pretend to return `int` from `die_push_simple()`
"Johannes Schindelin via GitGitGadget"
writes:
> From: Johannes Schindelin
>
> This function is marked as `NORETURN`, and it indeed does not want to
> return anything. So let's not declare it with the return type `int`.
> This fixes the following warning when building with MSVC:
>
> C4646: function declared with 'noreturn' has non-void return type
>
> Signed-off-by: Johannes Schindelin
> ---
> builtin/push.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 021dd3b1e4..d216270d5f 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -143,8 +143,8 @@ static int push_url_of_remote(struct remote *remote,
> const char ***url_p)
> return remote->url_nr;
> }
>
> -static NORETURN int die_push_simple(struct branch *branch,
> - struct remote *remote)
> +static NORETURN void die_push_simple(struct branch *branch,
> + struct remote *remote)
Haha ;-) "I won't return and I'll return int" sounds like an
oxymoron.
> {
> /*
>* There's no point in using shorten_unambiguous_ref here,
Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types
"Johannes Schindelin via GitGitGadget"
writes:
> While at it, we take care of reporting overflows (which are unlikely,
> but hey, defensive programming is good!).
>
> We _also_ take pains of casting the unsigned value to signed: otherwise,
> the signed operand (i.e. the `-1`) would be cast to unsigned before
> doing the arithmetic.
These three look good and too similar to each other, which makes me
wonder if we want to allow them simply write
return insert_pos_as_negative_offset(nr);
with something like
static int insert_pos_as_negative_offset(uintmax_t nr)
{
if (INT_MAX < nr)
die("overflow: -1 - %"PRIuMAX, nr);
return -1 - (int)nr;
}
to avoid repetition.
> Helped-by: Denton Liu
> Signed-off-by: Johannes Schindelin
> ---
> read-cache.c | 9 ++---
> sha1-lookup.c | 12 +---
> 2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index c701f7f8b8..97745c2a31 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1275,8 +1275,11 @@ static int add_index_entry_with_check(struct
> index_state *istate, struct cache_e
>* we can avoid searching for it.
>*/
> if (istate->cache_nr > 0 &&
> - strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
> - pos = -istate->cache_nr - 1;
> + strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) >
> 0) {
> + if (istate->cache_nr > INT_MAX)
> + die("overflow: -1 - %u", istate->cache_nr);
> + pos = -1 - (int)istate->cache_nr;
> + }
> else
> pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce),
> ce_stage(ce));
>
> @@ -1894,7 +1897,7 @@ static size_t estimate_cache_size(size_t ondisk_size,
> unsigned int entries)
> /*
>* Account for potential alignment differences.
>*/
> - per_entry += align_padding_size(sizeof(struct cache_entry),
> -sizeof(struct ondisk_cache_entry));
> + per_entry += align_padding_size(per_entry, 0);
> return ondisk_size + entries * per_entry;
> }
>
> diff --git a/sha1-lookup.c b/sha1-lookup.c
> index 796ab68da8..bb786b5420 100644
> --- a/sha1-lookup.c
> +++ b/sha1-lookup.c
> @@ -69,8 +69,12 @@ int sha1_pos(const unsigned char *sha1, void *table,
> size_t nr,
> miv = take2(sha1 + ofs);
> if (miv < lov)
> return -1;
> - if (hiv < miv)
> - return -1 - nr;
> + if (hiv < miv) {
> + if (nr > INT_MAX)
> + die("overflow: -1 - %"PRIuMAX,
> + (uintmax_t)nr);
> + return -1 - (int)nr;
> + }
> if (lov != hiv) {
> /*
>* At this point miv could be equal
> @@ -97,7 +101,9 @@ int sha1_pos(const unsigned char *sha1, void *table,
> size_t nr,
> lo = mi + 1;
> mi = lo + (hi - lo) / 2;
> } while (lo < hi);
> - return -lo-1;
> + if (nr > INT_MAX)
> + die("overflow: -1 - %"PRIuMAX, (uintmax_t)lo);
> + return -1 - (int)lo;
> }
>
> int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,
Re: [PATCH v3 0/6] multi-pack-index: add --no-progress
"William Baker via GitGitGadget" writes: > This is the third iteration of changes for adding support for > --[no-]progress to multi-pack-index, and it includes the following updates > for the feedback I received on v2: > > * Fixed commit message formatting > * Updated 'pack_paths_checked' from u32 to unsigned > > Thanks, William Thanks, will take a look.
Re: [BUG] incorrect line numbers reported in git am
Duy Nguyen writes: > On Thu, Oct 3, 2019 at 7:52 AM Junio C Hamano wrote: >> > In fact, running `git am --show-current-patch` shows the whole mail, not >> > only the 'patch' file so users would have no reason to expect the line >> > numbers to refer to the 'patch' file. >> >> Yeah, show-current-patch was a misguided attempt to hide useful >> information from the users. > > Not so much hiding as not having the information to present, at least > not the easy way, since the mail is split at the beginning of git-am > and never stored in $GIT_DIR. By the time this command is run, the > mail is already gone. Someone could of course update git-am to keep a > copy of the mail and improve this option. By "hiding", I meant "rob from the users an opportunity to learn where the useful patch file is stored". You seem to be doubly confused in this case, in that (1) you seem to have mistaken that I was complaining about show-current-patch not giving the full information contained in the original e-mail, and (2) you seem to think show-current-patch gives the contents of the patch witout other e-mail cruft. Both are incorrect. The first thing the command does is to feed the input to mailsplit and store the results in numbered files "%04d", and they are not removed until truly done. When you need to inspect the patch that does not apply, they are still there. Even emails for those steps that have been successfully applied before the current one are also there (the split files are all gone, though, but they no longer matter as they have been applied fine). I wouldn't have been so critical if "git am --show-current-patch" were implemented as "cat $GIT_DIR/rebase-apply/patch", but it does an equivalent of "cd $GIT_DIR/rebase-apply; cat $(cat next)" which is much less useful when trying to fix up the patch text that does not apply.
Re: [RFC PATCH v5 2/3] grep: make PCRE2 aware of custom allocator
Carlo Arenas writes: > or we could drop the current branch in pu and start again from scratch > now that all of the required dependencies had been merged. That would be the cleanest from my point of view. Thanks, both.
Re: PATCH] remove duplicate #include directives
René Scharfe writes:
> Found with "git grep '^#include ' '*.c' | sort | uniq -d".
>
> Signed-off-by: René Scharfe
> ---
> Patch formatted with --function-context for easier review.
I have a mixed feelings about that.
The only audience benefitted by --function-context patch are those
who read the patch only in MUA without looking at anything outside
and declare they are done with reviewing the patch. For something
tricky, wider context does help to see what is going on, but then
I'd feel uncomfortable to hear "looks good to me" from those who did
not even apply the patch to their trees and looked at the changes
after "reviewing" tricky stuff that requires wider context to
properly review.
If there were topics in flight that touch any of these include
blocks, the patch would not apply and a reviewer who is interested
in these fixes ends up needing to wiggle them in somehow. If a
reviewer does not trust you enough to feel the need to double check,
the result after applying the patches would be checked by running
"diff --function-context" by the reviewer (if it is tricky and
benefits from wider context) anyway. If you've earned enough trust
that such a verification is not needed, the reviewer may not need to
see --function-context output. So a patch that has less chance of
unnecessary conflict would be easier to handle in either case.
Having said all that, for _this_ particular case, I do not see a
reason why a review needs to look at anywhere outside the context
presented in this patch, so I'd say it is a narrow case that -W is
an appropriate thing to use. I just do not want to see contributors
less experienced than you (hence cannot make good judgement on when
to and not to use the option) get in the habit of using -W all the
time.
And needless to say, the changes in the patch look good.
Thanks.
> diff --git a/builtin/am.c b/builtin/am.c
> index ee7305eaa6..b015e1d7d1 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1,40 +1,39 @@
> /*
> * Builtin "git am"
> *
> * Based on git-am.sh by Junio C Hamano.
> */
> #define USE_THE_INDEX_COMPATIBILITY_MACROS
> #include "cache.h"
> #include "config.h"
> #include "builtin.h"
> #include "exec-cmd.h"
> #include "parse-options.h"
> #include "dir.h"
> #include "run-command.h"
> #include "quote.h"
> #include "tempfile.h"
> #include "lockfile.h"
> #include "cache-tree.h"
> #include "refs.h"
> #include "commit.h"
> #include "diff.h"
> #include "diffcore.h"
> #include "unpack-trees.h"
> #include "branch.h"
> #include "sequencer.h"
> #include "revision.h"
> #include "merge-recursive.h"
> -#include "revision.h"
> #include "log-tree.h"
> #include "notes-utils.h"
> #include "rerere.h"
> #include "prompt.h"
> #include "mailinfo.h"
> #include "apply.h"
> #include "string-list.h"
> #include "packfile.h"
> #include "repository.h"
>
> /**
> * Returns the length of the first line of msg.
> */
> diff --git a/builtin/blame.c b/builtin/blame.c
> index bfd537b344..9858d6b269 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1,32 +1,31 @@
> /*
> * Blame
> *
> * Copyright (c) 2006, 2014 by its authors
> * See COPYING for licensing conditions
> */
>
> #include "cache.h"
> #include "config.h"
> #include "color.h"
> #include "builtin.h"
> #include "repository.h"
> #include "commit.h"
> #include "diff.h"
> #include "revision.h"
> #include "quote.h"
> #include "string-list.h"
> #include "mailmap.h"
> #include "parse-options.h"
> #include "prio-queue.h"
> #include "utf8.h"
> #include "userdiff.h"
> #include "line-range.h"
> #include "line-log.h"
> #include "dir.h"
> #include "progress.h"
> #include "object-store.h"
> #include "blame.h"
> -#include "string-list.h"
> #include "refs.h"
>
> static char blame_usage[] = N_("git blame [] [] []
> [--] ");
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 2048b6760a..9d73102c42 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1,44 +1,43 @@
> /*
> * Builtin "git clone"
> *
> * Copyright (c) 2007 Kristian Høgsberg ,
> *2008 Daniel Barkalow
> * Based on git-commit.sh by Junio C Hamano and Linus Torvalds
> *
> * Clone a repository into a different directory that does not yet exist.
> */
>
> #define USE_THE_INDEX_COMPATIBILITY_MACROS
> #include "builtin.h"
> #include "config.h"
> #include "lockfile.h"
> #include "parse-options.h"
> #include "fetch-pack.h"
> #include "refs.h"
> #include "refspec.h"
> #include "object-store.h"
> #include "tree.h"
> #include "tree-walk.h"
> #include "unpack-trees.h"
> #include "transport.h"
> #include "strbuf.h"
> #include "dir.h"
> #include "dir-iterator.h"
> #include "iterator.h"
> #include "sigchain.h"
> #include "branch.h"
> #include "remote.h"
> #include "run-command.h"
> #include "connected.h"
> #include "packfile.h"
> #include "list-objects-filter-options.h"
> -#include "object-store.h"
>
> /*
> * Overall FIXMEs:
> * - respect DB_ENVIRONM
Re: [PATCH 0/1] fsmonitor: don't fill bitmap with entries to be removed
Johannes Schindelin writes: > Hi all, > > On Thu, 3 Oct 2019, William Baker via GitGitGadget wrote: > >> This patch series fixes a segfault that I encountered while testing >> fsmonitor. Under some circumstances, the fsmonitor extension was being >> written with too many bits, and subsequent git commands would segfault when >> trying to apply the bits to the index. >> >> As part of these changes I've added some BUG checks that would have helped >> catch this problem sooner. Special thanks to Dscho for pointing me in the >> right direction and suggesting a test that can reproduce the issue. >> >> Thanks, William > > Please note that I was involved with the development of this patch, > reviewed a couple of iterations internally and am implictly okay with > this first public iteration. IOW, "Reviewed-by: dscho". That's good to hear. THanks.
[PATCH v4 4/4] trace2: write overload message to sentinel files
Add a new "overload" event type for trace2 event destinations. When the
trace2 overload feature creates a sentinel file, it will include the
normal trace2 output in the sentinel, along with this new overload
event.
Writing this message into the sentinel file is useful for tracking how
often the overload protection feature is triggered in practice.
Bump up the event format version since we've added a new event type.
Signed-off-by: Josh Steadmon
---
Documentation/technical/api-trace2.txt | 15 ++--
t/t0212-trace2-event.sh| 4 ++-
trace2/tr2_dst.c | 47 ++
trace2/tr2_dst.h | 1 +
trace2/tr2_tgt_event.c | 31 +
trace2/tr2_tgt_normal.c| 2 +-
trace2/tr2_tgt_perf.c | 2 +-
7 files changed, 69 insertions(+), 33 deletions(-)
diff --git a/Documentation/technical/api-trace2.txt
b/Documentation/technical/api-trace2.txt
index 18b79128fd..5afd643fa6 100644
--- a/Documentation/technical/api-trace2.txt
+++ b/Documentation/technical/api-trace2.txt
@@ -128,7 +128,7 @@ yields
$ cat ~/log.event
-{"event":"version","sid":"sid":"20190408T191610.507018Z-H9b68c35f-P59a8","thread":"main","time":"2019-01-16T17:28:42.620713Z","file":"common-main.c","line":38,"evt":"1","exe":"2.20.1.155.g426c96fcdb"}
+{"event":"version","sid":"sid":"20190408T191610.507018Z-H9b68c35f-P59a8","thread":"main","time":"2019-01-16T17:28:42.620713Z","file":"common-main.c","line":38,"evt":"2","exe":"2.20.1.155.g426c96fcdb"}
{"event":"start","sid":"20190408T191610.507018Z-H9b68c35f-P59a8","thread":"main","time":"2019-01-16T17:28:42.621027Z","file":"common-main.c","line":39,"t_abs":0.001173,"argv":["git","version"]}
{"event":"cmd_name","sid":"20190408T191610.507018Z-H9b68c35f-P59a8","thread":"main","time":"2019-01-16T17:28:42.621122Z","file":"git.c","line":432,"name":"version","hierarchy":"version"}
{"event":"exit","sid":"20190408T191610.507018Z-H9b68c35f-P59a8","thread":"main","time":"2019-01-16T17:28:42.621236Z","file":"git.c","line":662,"t_abs":0.001227,"code":0}
@@ -616,11 +616,22 @@ only present on the "start" and "atexit" events.
{
"event":"version",
...
- "evt":"1", # EVENT format version
+ "evt":"2", # EVENT format version
"exe":"2.20.1.155.g426c96fcdb" # git version
}
+`"overload"`::
+ This event is created in a sentinel file if we are overloading a target
+ trace directory (see the trace2.maxFiles config option).
++
+
+{
+ "event":"overload",
+ ...
+}
+
+
`"start"`::
This event contains the complete argv received by main().
+
diff --git a/t/t0212-trace2-event.sh b/t/t0212-trace2-event.sh
index 2ff97e72da..7081153a11 100755
--- a/t/t0212-trace2-event.sh
+++ b/t/t0212-trace2-event.sh
@@ -279,7 +279,9 @@ test_expect_success "don't overload target directory" '
) &&
echo git-trace2-overload >>expected_filenames.txt &&
ls trace_target_dir >ls_output.txt &&
- test_cmp expected_filenames.txt ls_output.txt
+ test_cmp expected_filenames.txt ls_output.txt &&
+ head -n1 trace_target_dir/git-trace2-overload | grep
\"event\":\"version\" &&
+ head -n2 trace_target_dir/git-trace2-overload | tail -n1 | grep
\"event\":\"overload\"
'
test_done
diff --git a/trace2/tr2_dst.c b/trace2/tr2_dst.c
index af3405f179..eedc5a332f 100644
--- a/trace2/tr2_dst.c
+++ b/trace2/tr2_dst.c
@@ -50,15 +50,19 @@ void tr2_dst_trace_disable(struct tr2_dst *dst)
/*
* Check to make sure we're not overloading the target directory with too many
* files. First get the threshold (if present) from the config or envvar. If
- * it's zero or unset, disable this check. Next check for the presence of a
- * sentinel file, then check file count. If we are overloaded, create the
- * sentinel file if it doesn't already exist.
+ * it's zero or unset, disable this check. Next check for the presence of a
+ * sentinel file, then check file count.
+ *
+ * Returns 0 if tracing should proceed as normal. Returns 1 if the sentinel
file
+ * already exists, which means tracing should be disabled. Returns -1 if we are
+ * overloaded but there was no sentinel file, which means we have created and
+ * should write traces to the sentinel file.
*
* We expect that some trace processing system is gradually collecting files
* from the target directory; after it removes the sentinel file we'll start
* writing traces again.
*/
-static int tr2_dst_overloaded(const char *tgt_prefix)
+static int tr2_dst_overloaded(struct tr2_dst *dst, const char *tgt_prefix)
{
int file_count = 0, max_files = 0, ret = 0;
const char *max_files_var;
@@ -97,8 +101,9 @@ static int tr2_dst_overloaded(const char *tgt_prefix)
closedir(dirp);
if (file_count >= tr2env_max_files) {
-
[PATCH v4 0/4] trace2: don't overload target directories
Apologies for the delayed reply. I've added a new documentation patch
describing what we expect w.r.t. the version event and its "evt" field.
I've also reworked the final patch to allow writing a full trace session
to the sentinel file, and to make sure that the overload event comes
after the version event.
I am still incrementing the EVT and writing a new "overload" event,
since you said in the last review that this approache seems best to you.
But I am also happy to keep the EVT=1 and make "overload" a new field in
the "version" event if you've changed your mind.
Josh Steadmon (4):
docs: mention trace2 target-dir mode in git-config
docs: clarify trace2 version invariants
trace2: don't overload target directories
trace2: write overload message to sentinel files
Documentation/config/trace2.txt| 6 ++
Documentation/technical/api-trace2.txt | 30 +--
Documentation/trace2-target-values.txt | 4 +-
t/t0212-trace2-event.sh| 19 +
trace2/tr2_dst.c | 113 ++---
trace2/tr2_dst.h | 1 +
trace2/tr2_sysenv.c| 3 +
trace2/tr2_sysenv.h| 2 +
trace2/tr2_tgt_event.c | 31 +--
trace2/tr2_tgt_normal.c| 2 +-
trace2/tr2_tgt_perf.c | 2 +-
11 files changed, 185 insertions(+), 28 deletions(-)
Range-diff against v3:
-: -- > 1: a757bca8f9 docs: clarify trace2 version invariants
1: bf20ec8ea2 ! 2: 98a8440d3f trace2: don't overload target directories
@@ Commit message
The config can also be overridden with a new environment variable:
GIT_TRACE2_MAX_FILES.
-Potential future work:
-* Write a message into the sentinel file (should match the requested
- trace2 output format).
-* Add a performance test to make sure that contention between multiple
- processes all writing to the same target directory does not become an
- issue.
-
## Documentation/config/trace2.txt ##
@@ trace2/tr2_dst.c: void tr2_dst_trace_disable(struct tr2_dst *dst)
+ struct strbuf path = STRBUF_INIT, sentinel_path = STRBUF_INIT;
+ struct stat statbuf;
+
-+ strbuf_addstr(&path, tgt_prefix);
-+ if (!is_dir_sep(path.buf[path.len - 1])) {
-+ strbuf_addch(&path, '/');
-+ }
-+
+ /* Get the config or envvar and decide if we should continue this check
*/
+ max_files_var = tr2_sysenv_get(TR2_SYSENV_MAX_FILES);
+ if (max_files_var && *max_files_var && ((max_files =
atoi(max_files_var)) >= 0))
@@ trace2/tr2_dst.c: void tr2_dst_trace_disable(struct tr2_dst *dst)
+ goto cleanup;
+ }
+
++ strbuf_addstr(&path, tgt_prefix);
++ if (!is_dir_sep(path.buf[path.len - 1])) {
++ strbuf_addch(&path, '/');
++ }
++
+ /* check sentinel */
+ strbuf_addbuf(&sentinel_path, &path);
+ strbuf_addstr(&sentinel_path, OVERLOAD_SENTINEL_NAME);
2: bab45cb735 < -: -- trace2: write overload message to sentinel files
-: -- > 3: 790c5ac95a trace2: write overload message to sentinel files
--
2.23.0.581.g78d2f28ef7-goog
[PATCH v4 1/4] docs: mention trace2 target-dir mode in git-config
Move the description of trace2's target-directory behavior into the shared trace2-target-values file so that it is included in both the git-config and api-trace2 docs. Leave the SID discussion only in api-trace2 since it's a technical detail. Signed-off-by: Josh Steadmon --- Documentation/technical/api-trace2.txt | 7 +++ Documentation/trace2-target-values.txt | 4 +++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt index 71eb081fed..80ffceada0 100644 --- a/Documentation/technical/api-trace2.txt +++ b/Documentation/technical/api-trace2.txt @@ -142,10 +142,9 @@ system or global config value to one of the following: include::../trace2-target-values.txt[] -If the target already exists and is a directory, the traces will be -written to files (one per process) underneath the given directory. They -will be named according to the last component of the SID (optionally -followed by a counter to avoid filename collisions). +When trace files are written to a target directory, they will be named according +to the last component of the SID (optionally followed by a counter to avoid +filename collisions). == Trace2 API diff --git a/Documentation/trace2-target-values.txt b/Documentation/trace2-target-values.txt index 27d3c64e66..3985b6d3c2 100644 --- a/Documentation/trace2-target-values.txt +++ b/Documentation/trace2-target-values.txt @@ -2,7 +2,9 @@ * `0` or `false` - Disables the target. * `1` or `true` - Writes to `STDERR`. * `[2-9]` - Writes to the already opened file descriptor. -* `` - Writes to the file in append mode. +* `` - Writes to the file in append mode. If the target +already exists and is a directory, the traces will be written to files (one +per process) underneath the given directory. * `af_unix:[:]` - Write to a Unix DomainSocket (on platforms that support them). Socket type can be either `stream` or `dgram`; if omitted Git will -- 2.23.0.581.g78d2f28ef7-goog
[PATCH v4 2/4] docs: clarify trace2 version invariants
Make it explicit that we always want trace2 "version" events to be the
first event of any trace session. Also list the changes that would or
would not cause the EVENT format version field to be incremented.
Signed-off-by: Josh Steadmon
---
Documentation/technical/api-trace2.txt | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/Documentation/technical/api-trace2.txt
b/Documentation/technical/api-trace2.txt
index 80ffceada0..18b79128fd 100644
--- a/Documentation/technical/api-trace2.txt
+++ b/Documentation/technical/api-trace2.txt
@@ -604,7 +604,13 @@ only present on the "start" and "atexit" events.
Event-Specific Key/Value Pairs
`"version"`::
- This event gives the version of the executable and the EVENT format.
+ This event gives the version of the executable and the EVENT format. It
+ should always be the first event in a trace session. The EVENT format
+ version will be incremented if new event types are added, if existing
+ fields are removed, or if there are significant changes in
+ interpretation of existing events or fields. Smaller changes, such as
+ adding a new field to an existing event, will not require an increment
+ to the EVENT format version.
+
{
--
2.23.0.581.g78d2f28ef7-goog
[PATCH v4 3/4] trace2: don't overload target directories
trace2 can write files into a target directory. With heavy usage, this
directory can fill up with files, causing difficulty for
trace-processing systems.
This patch adds a config option (trace2.maxFiles) to set a maximum
number of files that trace2 will write to a target directory. The
following behavior is enabled when the maxFiles is set to a positive
integer:
When trace2 would write a file to a target directory, first check
whether or not the directory is overloaded. A directory is overloaded
if there is a sentinel file declaring an overload, or if the number of
files exceeds trace2.maxFiles. If the latter, create a sentinel file
to speed up later overload checks.
The assumption is that a separate trace-processing system is dealing
with the generated traces; once it processes and removes the sentinel
file, it should be safe to generate new trace files again.
The default value for trace2.maxFiles is zero, which disables the
overload check.
The config can also be overridden with a new environment variable:
GIT_TRACE2_MAX_FILES.
Signed-off-by: Josh Steadmon
---
Documentation/config/trace2.txt | 6 +++
t/t0212-trace2-event.sh | 17 +++
trace2/tr2_dst.c| 86 +
trace2/tr2_sysenv.c | 3 ++
trace2/tr2_sysenv.h | 2 +
5 files changed, 114 insertions(+)
diff --git a/Documentation/config/trace2.txt b/Documentation/config/trace2.txt
index 2edbfb02fe..4ce0b9a6d1 100644
--- a/Documentation/config/trace2.txt
+++ b/Documentation/config/trace2.txt
@@ -54,3 +54,9 @@ trace2.destinationDebug::
By default, these errors are suppressed and tracing is
silently disabled. May be overridden by the
`GIT_TRACE2_DST_DEBUG` environment variable.
+
+trace2.maxFiles::
+ Integer. When writing trace files to a target directory, do not
+ write additional traces if we would exceed this many files. Instead,
+ write a sentinel file that will block further tracing to this
+ directory. Defaults to 0, which disables this check.
diff --git a/t/t0212-trace2-event.sh b/t/t0212-trace2-event.sh
index ff5b9cc729..2ff97e72da 100755
--- a/t/t0212-trace2-event.sh
+++ b/t/t0212-trace2-event.sh
@@ -265,4 +265,21 @@ test_expect_success JSON_PP 'using global config, event
stream, error event' '
test_cmp expect actual
'
+test_expect_success "don't overload target directory" '
+ mkdir trace_target_dir &&
+ test_when_finished "rm -r trace_target_dir" &&
+ (
+ GIT_TRACE2_MAX_FILES=5 &&
+ export GIT_TRACE2_MAX_FILES &&
+ cd trace_target_dir &&
+ test_seq $GIT_TRACE2_MAX_FILES >../expected_filenames.txt &&
+ xargs touch <../expected_filenames.txt &&
+ cd .. &&
+ GIT_TRACE2_EVENT="$(pwd)/trace_target_dir" test-tool trace2
001return 0
+ ) &&
+ echo git-trace2-overload >>expected_filenames.txt &&
+ ls trace_target_dir >ls_output.txt &&
+ test_cmp expected_filenames.txt ls_output.txt
+'
+
test_done
diff --git a/trace2/tr2_dst.c b/trace2/tr2_dst.c
index 5dda0ca1cd..af3405f179 100644
--- a/trace2/tr2_dst.c
+++ b/trace2/tr2_dst.c
@@ -1,3 +1,5 @@
+#include
+
#include "cache.h"
#include "trace2/tr2_dst.h"
#include "trace2/tr2_sid.h"
@@ -8,6 +10,19 @@
*/
#define MAX_AUTO_ATTEMPTS 10
+/*
+ * Sentinel file used to detect when we're overloading a directory with too
many
+ * trace files.
+ */
+#define OVERLOAD_SENTINEL_NAME "git-trace2-overload"
+
+/*
+ * When set to zero, disables directory overload checking. Otherwise, controls
+ * how many files we can write to a directory before entering overload mode.
+ * This can be overridden via the TR2_SYSENV_MAX_FILES setting.
+ */
+static int tr2env_max_files = 0;
+
static int tr2_dst_want_warning(void)
{
static int tr2env_dst_debug = -1;
@@ -32,6 +47,67 @@ void tr2_dst_trace_disable(struct tr2_dst *dst)
dst->need_close = 0;
}
+/*
+ * Check to make sure we're not overloading the target directory with too many
+ * files. First get the threshold (if present) from the config or envvar. If
+ * it's zero or unset, disable this check. Next check for the presence of a
+ * sentinel file, then check file count. If we are overloaded, create the
+ * sentinel file if it doesn't already exist.
+ *
+ * We expect that some trace processing system is gradually collecting files
+ * from the target directory; after it removes the sentinel file we'll start
+ * writing traces again.
+ */
+static int tr2_dst_overloaded(const char *tgt_prefix)
+{
+ int file_count = 0, max_files = 0, ret = 0;
+ const char *max_files_var;
+ DIR *dirp;
+ struct strbuf path = STRBUF_INIT, sentinel_path = STRBUF_INIT;
+ struct stat statbuf;
+
+ /* Get the config or envvar and decide if we should continue this check
*/
+ max_files_var = tr2_sysenv_get(TR2_SYSENV_MAX_FILES);
+ if (max_files_var
Re: [PATCH 1/1] fsmonitor: don't fill bitmap with entries to be removed
"William Baker via GitGitGadget" writes:
> create mode 100755 t/t7519/fsmonitor-env
> ...
> + if (pos >= istate->cache_nr)
> + BUG("fsmonitor_dirty has more entries than the index
> (%"PRIuMAX" >= %"PRIuMAX")",
> + (uintmax_t)pos, (uintmax_t)istate->cache_nr);
This is how we show size_t values without using "%z" that we avoid,
but are "pos" and 'cache_nr" size_t or ssize_t? I thought they are
plain boring unsigned, so shouldn't we use the plain boring "%u"
without casting?
The same comment applies to other uses of uintmax_t cast in this
patch.
> void fill_fsmonitor_bitmap(struct index_state *istate)
> {
> - unsigned int i;
> + unsigned int i, skipped = 0;
> istate->fsmonitor_dirty = ewah_new();
> - for (i = 0; i < istate->cache_nr; i++)
> - if (!(istate->cache[i]->ce_flags & CE_FSMONITOR_VALID))
> - ewah_set(istate->fsmonitor_dirty, i);
> + for (i = 0; i < istate->cache_nr; i++) {
> + if (istate->cache[i]->ce_flags & CE_REMOVE)
> + skipped++;
> + else if (!(istate->cache[i]->ce_flags & CE_FSMONITOR_VALID))
> + ewah_set(istate->fsmonitor_dirty, i - skipped);
> + }
> }
Matches the explanation in the proposed log message pretty well.
Good job.
> @@ -354,4 +354,16 @@ test_expect_success 'discard_index() also discards
> fsmonitor info' '
> test_cmp expect actual
> '
>
> +# Use test files that start with 'z' so that the entries being added
> +# and removed appear at the end of the index.
In other words, future developers are warned against adding entries
to and leaving them in the index that sort later than z100 in new
tests they add before this point. Is the above wording clear enough
to tell them that, I wonder?
> +test_expect_success 'status succeeds after staging/unstaging ' '
> + test_commit initial &&
> + removed=$(test_seq 1 100 | sed "s/^/z/") &&
Thanks.
[PATCH v2 2/5] t4214: use test_merge
In the previous commit, we extended test_merge() so that it could perform octopus merges. Now that the restriction is lifted, use test_merge() to perform the octopus merge instead of manually duplicating test_merge() functionality. Signed-off-by: Denton Liu --- t/t4214-log-graph-octopus.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh index dab96c89aa..f6e22ec825 100755 --- a/t/t4214-log-graph-octopus.sh +++ b/t/t4214-log-graph-octopus.sh @@ -41,8 +41,7 @@ test_expect_success 'set up merge history' ' test_commit $i $i $i tag$i || return $? done && git checkout 1 -b merge && - test_tick && - git merge -m octopus-merge 1 2 3 4 && + test_merge octopus-merge 1 2 3 4 && git checkout 1 -b L && test_commit left ' -- 2.23.0.565.g1cc52d20df
[PATCH v2 0/5] t4214: cleanup and demonstrate graph bug
Junio, the test cases from earlier didn't exactly cover the cases Peff talked about so I added a few more test cases. These should cover those situations and a few more so we can be extra sure when the bug is fixed. This patchset cleans up t4214 and then, in the last patch, demonstrates a bug in the way some octopus merges are colored. While I was playing around with Pratyush's octopus merge in git-gui, I noticed that there was a bug in `git log --graph`. The horizontal lines in the octopus merge seemed to have an off-by-one error in their coloring. More detail in the last patch. I tried my hand at fixing the bug but in the hour I spent going at it, I couldn't fix the logic up. The buggy logic is in graph.c: graph_draw_octopus_merge() in case anyone is interested. Changes since v1: * Add a few more test cases to demonstrate more failure (and passing) conditions Denton Liu (5): test-lib: let test_merge() perform octopus merges t4214: use test_merge t4214: generate expect in their own test cases t4214: explicitly list tags in log t4214: demonstrate octopus graph coloring failure t/t4214-log-graph-octopus.sh | 329 --- t/test-lib-functions.sh | 6 +- 2 files changed, 308 insertions(+), 27 deletions(-) Range-diff against v1: 1: e77af8cde5 = 1: e77af8cde5 test-lib: let test_merge() perform octopus merges 2: 4a93deb3f6 = 2: 4a93deb3f6 t4214: use test_merge 3: c09f761185 = 3: c09f761185 t4214: generate expect in their own test cases 4: ad6d89440b = 4: ad6d89440b t4214: explicitly list tags in log 5: 0b84bf5417 ! 5: e58c1929bc t4214: demonstrate octopus graph coloring failure @@ Commit message dash should be the color of the line to their bottom-right. Instead, they are currently the color of the line to their bottom. -Demonstrate this breakage with two sets of test cases. The first pair of -test cases demonstrates the breakage with a similar case as the above. -The second pair of test cases demonstrates a similar breakage but with -the last parent crossing over. +Demonstrate this breakage with a few sets of test cases. These test +cases should show not only simple cases of the bug occuring but trickier +situations that may not be handled properly in any attempt to fix the +bug. -The second pair of test cases are included as a result of my (poor) -attempts at fixing the bug. This case seems particularly tricky to -handle. Good luck! +While we're at it, include a passing test case as a canary in case an +attempt to fix the bug breaks existing operation. ## t/t4214-log-graph-octopus.sh ## @@ t/t4214-log-graph-octopus.sh: test_expect_success 'set up merge history' ' @@ t/t4214-log-graph-octopus.sh: test_expect_success 'set up merge history' ' - test_commit left + test_commit left && + git checkout 4 -b crossover && -+ test_commit after-4 ++ test_commit after-4 && ++ git checkout initial -b more-L && ++ test_commit after-initial ' test_expect_success 'log --graph with tricky octopus merge, no color' ' @@ t/t4214-log-graph-octopus.sh: test_expect_success 'log --graph with normal octop test_cmp expect.colors actual.colors ' + -+test_expect_success 'log --graph with tricky octopus merge and its parent, no color' ' ++test_expect_success 'log --graph with normal octopus merge and child, no color' ' ++ cat >expect.uncolored <<-\EOF && ++ * after-merge ++ *---. octopus-merge ++ |\ \ \ ++ | | | * 4 ++ | | * | 3 ++ | | |/ ++ | * | 2 ++ | |/ ++ * | 1 ++ |/ ++ * initial ++ EOF ++ git log --color=never --graph --date-order --pretty=tformat:%s after-merge >actual.raw && ++ sed "s/ *\$//" actual.raw >actual && ++ test_cmp expect.uncolored actual ++' ++ ++test_expect_failure 'log --graph with normal octopus and child merge with colors' ' ++ cat >expect.colors <<-\EOF && ++ * after-merge ++ *---. octopus-merge ++ |\ \ \ ++ | | | * 4 ++ | | * | 3 ++ | | |/ ++ | * | 2 ++ | |/ ++ * | 1 ++ |/ ++ * initial ++ EOF ++ test_config log.graphColors red,green,yellow,blue,magenta,cyan && ++ git log --color=always --graph --date-order --pretty=tformat:%s after-merge >actual.colors.raw && ++ test_decode_color actual.colors && ++ test_cmp expect.colors actual.colors ++' ++ ++test_expect_success 'log --graph with tricky octopus merge and its child, no color' ' + cat >expect.uncolored <<-\EOF && + * left + | * after-merge @@ t/t4214-log-graph-octopus.sh: test_expect_success 'log --graph with normal octop + test_cmp expect.uncolored actual +' + -+test_expect_failure 'log --graph with tricky o
[PATCH v2 4/5] t4214: explicitly list tags in log
In a future test case, we will be extending the commit graph. As a result, explicitly list the tags that will generate the graph so that when future additions are made, the current graph illustrations won't be affected. Signed-off-by: Denton Liu --- t/t4214-log-graph-octopus.sh | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh index 16776e347c..097151da39 100755 --- a/t/t4214-log-graph-octopus.sh +++ b/t/t4214-log-graph-octopus.sh @@ -33,7 +33,7 @@ test_expect_success 'log --graph with tricky octopus merge, no color' ' |/ * initial EOF - git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw && + git log --color=never --graph --date-order --pretty=tformat:%s left octopus-merge >actual.raw && sed "s/ *\$//" actual.raw >actual && test_cmp expect.uncolored actual ' @@ -54,7 +54,7 @@ test_expect_success 'log --graph with tricky octopus merge with colors' ' |/ * initial EOF - git log --color=always --graph --date-order --pretty=tformat:%s --all >actual.colors.raw && + git log --color=always --graph --date-order --pretty=tformat:%s left octopus-merge >actual.colors.raw && test_decode_color actual.colors && test_cmp expect.colors actual.colors ' @@ -75,7 +75,7 @@ test_expect_success 'log --graph with normal octopus merge, no color' ' |/ * initial EOF - git log --color=never --graph --date-order --pretty=tformat:%s merge >actual.raw && + git log --color=never --graph --date-order --pretty=tformat:%s octopus-merge >actual.raw && sed "s/ *\$//" actual.raw >actual && test_cmp expect.uncolored actual ' @@ -94,7 +94,7 @@ test_expect_success 'log --graph with normal octopus merge with colors' ' * initial EOF test_config log.graphColors red,green,yellow,blue,magenta,cyan && - git log --color=always --graph --date-order --pretty=tformat:%s merge >actual.colors.raw && + git log --color=always --graph --date-order --pretty=tformat:%s octopus-merge >actual.colors.raw && test_decode_color actual.colors && test_cmp expect.colors actual.colors ' -- 2.23.0.565.g1cc52d20df
[PATCH v2 5/5] t4214: demonstrate octopus graph coloring failure
The graph coloring logic for octopus merges currently has a bug. This can be seen git.git with 74c7cfa875 (Merge of http://members.cox.net/junkio/git-jc.git, 2005-05-05), whose second child is 211232bae6 (Octopus merge of the following five patches., 2005-05-05). If one runs git log --graph 74c7cfa875 one can see that the octopus merge is colored incorrectly. In particular, the horizontal dashes are off by one color. Each horizontal dash should be the color of the line to their bottom-right. Instead, they are currently the color of the line to their bottom. Demonstrate this breakage with a few sets of test cases. These test cases should show not only simple cases of the bug occuring but trickier situations that may not be handled properly in any attempt to fix the bug. While we're at it, include a passing test case as a canary in case an attempt to fix the bug breaks existing operation. Signed-off-by: Denton Liu --- t/t4214-log-graph-octopus.sh | 282 ++- 1 file changed, 281 insertions(+), 1 deletion(-) diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh index 097151da39..3ae8e51e50 100755 --- a/t/t4214-log-graph-octopus.sh +++ b/t/t4214-log-graph-octopus.sh @@ -14,8 +14,13 @@ test_expect_success 'set up merge history' ' done && git checkout 1 -b merge && test_merge octopus-merge 1 2 3 4 && + test_commit after-merge && git checkout 1 -b L && - test_commit left + test_commit left && + git checkout 4 -b crossover && + test_commit after-4 && + git checkout initial -b more-L && + test_commit after-initial ' test_expect_success 'log --graph with tricky octopus merge, no color' ' @@ -98,4 +103,279 @@ test_expect_success 'log --graph with normal octopus merge with colors' ' test_decode_color actual.colors && test_cmp expect.colors actual.colors ' + +test_expect_success 'log --graph with normal octopus merge and child, no color' ' + cat >expect.uncolored <<-\EOF && + * after-merge + *---. octopus-merge + |\ \ \ + | | | * 4 + | | * | 3 + | | |/ + | * | 2 + | |/ + * | 1 + |/ + * initial + EOF + git log --color=never --graph --date-order --pretty=tformat:%s after-merge >actual.raw && + sed "s/ *\$//" actual.raw >actual && + test_cmp expect.uncolored actual +' + +test_expect_failure 'log --graph with normal octopus and child merge with colors' ' + cat >expect.colors <<-\EOF && + * after-merge + *---. octopus-merge + |\ \ \ + | | | * 4 + | | * | 3 + | | |/ + | * | 2 + | |/ + * | 1 + |/ + * initial + EOF + test_config log.graphColors red,green,yellow,blue,magenta,cyan && + git log --color=always --graph --date-order --pretty=tformat:%s after-merge >actual.colors.raw && + test_decode_color actual.colors && + test_cmp expect.colors actual.colors +' + +test_expect_success 'log --graph with tricky octopus merge and its child, no color' ' + cat >expect.uncolored <<-\EOF && + * left + | * after-merge + | *---. octopus-merge + | |\ \ \ + |/ / / / + | | | * 4 + | | * | 3 + | | |/ + | * | 2 + | |/ + * | 1 + |/ + * initial + EOF + git log --color=never --graph --date-order --pretty=tformat:%s left after-merge >actual.raw && + sed "s/ *\$//" actual.raw >actual && + test_cmp expect.uncolored actual +' + +test_expect_failure 'log --graph with tricky octopus merge and its child with colors' ' + test_config log.graphColors red,green,yellow,blue,magenta,cyan && + cat >expect.colors <<-\EOF && + * left + | * after-merge + | *---. octopus-merge + | |\ \ \ + |/ / / / + | | | * 4 + | | * | 3 + | | |/ + | * | 2 + | |/ + * | 1 + |/ + * initial + EOF + git log --color=always --graph --date-order --pretty=tformat:%s left after-merge >actual.colors.raw && + test_decode_color actual.colors && + test_cmp expect.colors actual.colors +' + +test_expect_success 'log --graph with crossover in octopus merge, no color' ' + cat >expect.uncolored <<-\EOF && + * after-4 + | *---. octopus-merge + | |\ \ \ + | |_|_|/ + |/| | | + * | | | 4 + | | | * 3 + | |_|/ + |/| | + | | * 2 + | |/ + |/| + | * 1 + |/ + * initial + EOF + git log --color=never --graph --date-order --pretty=tformat:%s after-4 octopus-merge >actual.raw && + sed "s/ *\$//" actual.raw >actual && + test_cmp expect.uncolored actual +' + +test_expect_failure 'log --graph with crossover in octopus merge with colors' ' + test_config log.graphColors red,green,yellow,blue,magenta,c
[PATCH v2 1/5] test-lib: let test_merge() perform octopus merges
Currently test_merge() only allows developers to merge in one branch.
However, this restriction is artificial and there is no reason why it
needs to be this way.
Extend test_merge() to allow the specification of multiple branches so
that octopus merges can be performed.
Signed-off-by: Denton Liu
---
t/test-lib-functions.sh | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 87bf3a2287..b299ecc326 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -228,9 +228,11 @@ test_commit () {
# can be a tag pointing to the commit-to-merge.
test_merge () {
+ label="$1" &&
+ shift &&
test_tick &&
- git merge -m "$1" "$2" &&
- git tag "$1"
+ git merge -m "$label" "$@" &&
+ git tag "$label"
}
# Efficiently create commits, each with a unique number (from 1 to
--
2.23.0.565.g1cc52d20df
[PATCH v2 3/5] t4214: generate expect in their own test cases
Before, the expect files of the test case were being generated in the setup method. However, it would make more sense to generate these files within the test cases that actually use them so that it's obvious to future readers where the expected values are coming from. Move the generation of the expect files in their own respective test cases. While we're at it, we want to establish a pattern in this test suite that, firstly, a non-colored test case is given then, immediately after, the colored version is given. Switch test cases "log --graph with tricky octopus merge, no color" and "log --graph with tricky octopus merge with colors" so that the "no color" version appears first. This patch is best viewed with `--color-moved`. Signed-off-by: Denton Liu --- t/t4214-log-graph-octopus.sh | 42 ++-- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh index f6e22ec825..16776e347c 100755 --- a/t/t4214-log-graph-octopus.sh +++ b/t/t4214-log-graph-octopus.sh @@ -5,6 +5,20 @@ test_description='git log --graph of skewed left octopus merge.' . ./test-lib.sh test_expect_success 'set up merge history' ' + test_commit initial && + for i in 1 2 3 4 ; do + git checkout master -b $i || return $? + # Make tag name different from branch name, to avoid + # ambiguity error when calling checkout. + test_commit $i $i $i tag$i || return $? + done && + git checkout 1 -b merge && + test_merge octopus-merge 1 2 3 4 && + git checkout 1 -b L && + test_commit left +' + +test_expect_success 'log --graph with tricky octopus merge, no color' ' cat >expect.uncolored <<-\EOF && * left | *---. octopus-merge @@ -19,6 +33,13 @@ test_expect_success 'set up merge history' ' |/ * initial EOF + git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw && + sed "s/ *\$//" actual.raw >actual && + test_cmp expect.uncolored actual +' + +test_expect_success 'log --graph with tricky octopus merge with colors' ' + test_config log.graphColors red,green,yellow,blue,magenta,cyan && cat >expect.colors <<-\EOF && * left | *---. octopus-merge @@ -33,32 +54,11 @@ test_expect_success 'set up merge history' ' |/ * initial EOF - test_commit initial && - for i in 1 2 3 4 ; do - git checkout master -b $i || return $? - # Make tag name different from branch name, to avoid - # ambiguity error when calling checkout. - test_commit $i $i $i tag$i || return $? - done && - git checkout 1 -b merge && - test_merge octopus-merge 1 2 3 4 && - git checkout 1 -b L && - test_commit left -' - -test_expect_success 'log --graph with tricky octopus merge with colors' ' - test_config log.graphColors red,green,yellow,blue,magenta,cyan && git log --color=always --graph --date-order --pretty=tformat:%s --all >actual.colors.raw && test_decode_color actual.colors && test_cmp expect.colors actual.colors ' -test_expect_success 'log --graph with tricky octopus merge, no color' ' - git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw && - sed "s/ *\$//" actual.raw >actual && - test_cmp expect.uncolored actual -' - # Repeat the previous two tests with "normal" octopus merge (i.e., # without the first parent skewing to the "left" branch column). -- 2.23.0.565.g1cc52d20df
Re: [PATCH v4 3/4] trace2: don't overload target directories
Josh Steadmon writes:
> trace2 can write files into a target directory. With heavy usage, this
> directory can fill up with files, causing difficulty for
> trace-processing systems.
Sorry for not mentioning this, but "don't overload" is a suboptimal
keyword for the entire topic and for this particular step for a few
reasons. For one, "overload" is an overloaded verb that gives an
incorrect impression that the problem you are dealing with is that
the target directory you specify is (mis)used for other purposes,
which is not the case. You instead refrain from creating too many
files. The other (which is probably more serious) is that it is
unclear what approach you chose to solve the "directory ends up
holding too many files". One could simply discard new traces to do
so, one could concatenate to existing files to avoid creating new
files, one could even cycle the directory (i.e. path/to/log may
become path/to/log.old.1 and path/to/log is recreated as an empty
directory when it gets a new file).
trace2: discard new traces when a target directory has too many files
or something would convey the problem you are solving (i.e. "too
many files" implying negative performance and usability impact
coming from it) and solution (i.e. "discard new traces"), if it is
the approach you have chosen.
> + /* check sentinel */
> + strbuf_addbuf(&sentinel_path, &path);
> + strbuf_addstr(&sentinel_path, OVERLOAD_SENTINEL_NAME);
> + if (!stat(sentinel_path.buf, &statbuf)) {
> + ret = 1;
> + goto cleanup;
> + }
> +
> + /* check file count */
> + dirp = opendir(path.buf);
> + while (file_count < tr2env_max_files && dirp && readdir(dirp))
> + file_count++;
> + if (dirp)
> + closedir(dirp);
So, until we accumulate too many files in the directory, every
process when it starts tracing will scan the output directory.
Hopefully the max is not set to too large a value.
Re: [PATCH v2 18/19] OFFSETOF_VAR macro to simplify hashmap iterators
Eric Wong writes:
> In the future, list iterator macros (e.g. list_for_each_entry)
> may also be implemented using OFFSETOF_VAR to save hackers the
> trouble of using container_of/list_entry macros and without
> relying on non-portable `__typeof__'.
Can we add something like this as a preliminary preparation step
before the series?
Subject: [PATCH] treewide: initialize pointers to hashmap entries
There are not strictly necessary, but some compilers (e.g. clang
6.0.1) apparently have trouble in construct we will use in the
OFFSETOF_VAR() macro, i.e.
((uintptr_t)&(ptr)->member - (uintptr_t)(ptr))
when the ptr is uninitialized.
Signed-off-by: Junio C Hamano
---
attr.c | 2 +-
blame.c | 4 ++--
builtin/describe.c | 2 +-
builtin/difftool.c | 2 +-
config.c| 2 +-
merge-recursive.c | 6 +++---
revision.c | 4 ++--
submodule-config.c | 2 +-
t/helper/test-hashmap.c | 2 +-
t/helper/test-lazy-init-name-hash.c | 4 ++--
10 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/attr.c b/attr.c
index 15f0efdf60..0c367fb84a 100644
--- a/attr.c
+++ b/attr.c
@@ -161,7 +161,7 @@ static void all_attrs_init(struct attr_hashmap *map, struct
attr_check *check)
* field and fill each entry with its corresponding git_attr.
*/
if (size != check->all_attrs_nr) {
- struct attr_hash_entry *e;
+ struct attr_hash_entry *e = NULL;
struct hashmap_iter iter;
REALLOC_ARRAY(check->all_attrs, size);
diff --git a/blame.c b/blame.c
index 6384f48133..a5f8672419 100644
--- a/blame.c
+++ b/blame.c
@@ -448,7 +448,7 @@ static int fingerprint_similarity(struct fingerprint *a,
struct fingerprint *b)
{
int intersection = 0;
struct hashmap_iter iter;
- const struct fingerprint_entry *entry_a, *entry_b;
+ const struct fingerprint_entry *entry_a, *entry_b = NULL;
hashmap_for_each_entry(&b->map, &iter, entry_b,
entry /* member name */) {
@@ -467,7 +467,7 @@ static void fingerprint_subtract(struct fingerprint *a,
struct fingerprint *b)
{
struct hashmap_iter iter;
struct fingerprint_entry *entry_a;
- const struct fingerprint_entry *entry_b;
+ const struct fingerprint_entry *entry_b = NULL;
hashmap_iter_init(&b->map, &iter);
diff --git a/builtin/describe.c b/builtin/describe.c
index 1caf98f716..42e6cc3a4f 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -330,7 +330,7 @@ static void describe_commit(struct object_id *oid, struct
strbuf *dst)
if (!have_util) {
struct hashmap_iter iter;
struct commit *c;
- struct commit_name *n;
+ struct commit_name *n = NULL;
init_commit_names(&commit_names);
hashmap_for_each_entry(&names, &iter, n,
diff --git a/builtin/difftool.c b/builtin/difftool.c
index c280e682b2..30f3bd6219 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -344,7 +344,7 @@ static int run_dir_diff(const char *extcmd, int symlinks,
const char *prefix,
FILE *fp;
struct hashmap working_tree_dups, submodules, symlinks2;
struct hashmap_iter iter;
- struct pair_entry *entry;
+ struct pair_entry *entry = NULL;
struct index_state wtindex;
struct checkout lstate, rstate;
int rc, flags = RUN_GIT_CMD, err = 0;
diff --git a/config.c b/config.c
index a4fa464ed2..6ceefb7cff 100644
--- a/config.c
+++ b/config.c
@@ -1936,7 +1936,7 @@ void git_configset_init(struct config_set *cs)
void git_configset_clear(struct config_set *cs)
{
- struct config_set_element *entry;
+ struct config_set_element *entry = NULL;
struct hashmap_iter iter;
if (!cs->hash_initialized)
return;
diff --git a/merge-recursive.c b/merge-recursive.c
index 8787a40b0c..bc826fb7ac 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2127,7 +2127,7 @@ static void handle_directory_level_conflicts(struct
merge_options *opt,
struct tree *merge)
{
struct hashmap_iter iter;
- struct dir_rename_entry *head_ent;
+ struct dir_rename_entry *head_ent = NULL;
struct dir_rename_entry *merge_ent;
struct string_list remove_from_head = STRING_LIST_INIT_NODUP;
@@ -2566,7 +2566,7 @@ static struct string_list *get_renames(struct
merge_options *opt,
int i;
struct hashmap collisions;
struct hashmap_iter iter;
- struct collision_entry *e;
+ struct collision_entry *e = NULL;
struct string_list *renames;
compute_collisions(&collisions, dir_renames, pairs);
@@ -2838,7 +2838,7 @@ static void initial_cleanup_rename(struct
diff_queue_struct *pairs,
Re: [PATCH v2 18/19] OFFSETOF_VAR macro to simplify hashmap iterators
Junio C Hamano wrote: > Eric Wong writes: > > > In the future, list iterator macros (e.g. list_for_each_entry) > > may also be implemented using OFFSETOF_VAR to save hackers the > > trouble of using container_of/list_entry macros and without > > relying on non-portable `__typeof__'. > > Can we add something like this as a preliminary preparation step > before the series? > > Subject: [PATCH] treewide: initialize pointers to hashmap entries > > There are not strictly necessary, but some compilers (e.g. clang > 6.0.1) apparently have trouble in construct we will use in the > OFFSETOF_VAR() macro, i.e. > > ((uintptr_t)&(ptr)->member - (uintptr_t)(ptr)) > > when the ptr is uninitialized. > > Signed-off-by: Junio C Hamano > --- > attr.c | 2 +- > blame.c | 4 ++-- > builtin/describe.c | 2 +- > builtin/difftool.c | 2 +- > config.c| 2 +- > merge-recursive.c | 6 +++--- > revision.c | 4 ++-- > submodule-config.c | 2 +- > t/helper/test-hashmap.c | 2 +- > t/helper/test-lazy-init-name-hash.c | 4 ++-- > 10 files changed, 15 insertions(+), 15 deletions(-) That seems too tedious. I'm learning towards just initializing var = NULL in the start of the for-loop: @@ -449,7 +449,8 @@ static inline struct hashmap_entry *hashmap_iter_first(struct hashmap *map, * containing a @member which is a "struct hashmap_entry" */ #define hashmap_for_each_entry(map, iter, var, member) \ - for (var = hashmap_iter_first_entry_offset(map, iter, \ + for (var = NULL /* squelch uninitialized warnings for OFFSETOF_VAR */, \ + var = hashmap_iter_first_entry_offset(map, iter, \ OFFSETOF_VAR(var, member)); \ var; \ var = hashmap_iter_next_entry_offset(iter, \ (But I'm running on fumes all week, so not sure I trust it)
Re: [PATCH v2 18/19] OFFSETOF_VAR macro to simplify hashmap iterators
Eric Wong writes:
> That seems too tedious. I'm learning towards just initializing
> var = NULL in the start of the for-loop:
>
> @@ -449,7 +449,8 @@ static inline struct hashmap_entry
> *hashmap_iter_first(struct hashmap *map,
> * containing a @member which is a "struct hashmap_entry"
> */
> #define hashmap_for_each_entry(map, iter, var, member) \
> - for (var = hashmap_iter_first_entry_offset(map, iter, \
> + for (var = NULL /* squelch uninitialized warnings for OFFSETOF_VAR */, \
> + var = hashmap_iter_first_entry_offset(map, iter, \
> OFFSETOF_VAR(var, member)); \
That looks a bit too ugly for my taste. I've added an updated
version (see below) and then rebased the whole thing on top of it.
"hashmap: use *_entry APIs for iteration" needs a trivial textual
conflict resolved, but otherwise the result looked OK.
-- >8 --
From: Junio C Hamano
Date: Fri, 4 Oct 2019 10:12:25 +0900
Subject: [PATCH] treewide: initialize pointers to hashmap entries
There are not strictly necessary to avoid use of uninitialized
variables, but some compilers (e.g. clang 6.0.1) apparently have
trouble in construct we will use in the OFFSETOF_VAR() macro, i.e.
((uintptr_t)&(ptr)->member - (uintptr_t)(ptr))
when the ptr is uninitialized, treating such a "ptr" as "used".
It is just as bogus to subtract NULL from the address of the
"member" field pretending the structure sits at NULL address
as doing so with a structure that sits at a random address, but
apparently clang issues a warning with an advice to initialize the
pointer to NULL, so let's do that.
Signed-off-by: Junio C Hamano
---
attr.c | 2 +-
blame.c | 4 ++--
builtin/describe.c | 2 +-
builtin/difftool.c | 2 +-
config.c| 2 +-
merge-recursive.c | 6 +++---
revision.c | 4 ++--
submodule-config.c | 2 +-
t/helper/test-hashmap.c | 2 +-
t/helper/test-lazy-init-name-hash.c | 4 ++--
10 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/attr.c b/attr.c
index 93dc16b59c..cb85303a06 100644
--- a/attr.c
+++ b/attr.c
@@ -159,7 +159,7 @@ static void all_attrs_init(struct attr_hashmap *map, struct
attr_check *check)
* field and fill each entry with its corresponding git_attr.
*/
if (size != check->all_attrs_nr) {
- struct attr_hash_entry *e;
+ struct attr_hash_entry *e = NULL;
struct hashmap_iter iter;
hashmap_iter_init(&map->map, &iter);
diff --git a/blame.c b/blame.c
index 36a2e7ef11..9b912867f7 100644
--- a/blame.c
+++ b/blame.c
@@ -447,7 +447,7 @@ static int fingerprint_similarity(struct fingerprint *a,
struct fingerprint *b)
{
int intersection = 0;
struct hashmap_iter iter;
- const struct fingerprint_entry *entry_a, *entry_b;
+ const struct fingerprint_entry *entry_a, *entry_b = NULL;
hashmap_iter_init(&b->map, &iter);
@@ -466,7 +466,7 @@ static void fingerprint_subtract(struct fingerprint *a,
struct fingerprint *b)
{
struct hashmap_iter iter;
struct fingerprint_entry *entry_a;
- const struct fingerprint_entry *entry_b;
+ const struct fingerprint_entry *entry_b = NULL;
hashmap_iter_init(&b->map, &iter);
diff --git a/builtin/describe.c b/builtin/describe.c
index 200154297d..0369ed66ce 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -327,7 +327,7 @@ static void describe_commit(struct object_id *oid, struct
strbuf *dst)
if (!have_util) {
struct hashmap_iter iter;
struct commit *c;
- struct commit_name *n;
+ struct commit_name *n = NULL;
init_commit_names(&commit_names);
n = hashmap_iter_first(&names, &iter);
diff --git a/builtin/difftool.c b/builtin/difftool.c
index 16eb8b70ea..fbb4c87a86 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -337,7 +337,7 @@ static int run_dir_diff(const char *extcmd, int symlinks,
const char *prefix,
FILE *fp;
struct hashmap working_tree_dups, submodules, symlinks2;
struct hashmap_iter iter;
- struct pair_entry *entry;
+ struct pair_entry *entry = NULL;
struct index_state wtindex;
struct checkout lstate, rstate;
int rc, flags = RUN_GIT_CMD, err = 0;
diff --git a/config.c b/config.c
index 3900e4947b..6c92d2825d 100644
--- a/config.c
+++ b/config.c
@@ -1934,7 +1934,7 @@ void git_configset_init(struct config_set *cs)
void git_configset_clear(struct config_set *cs)
{
- struct config_set_element *entry;
+ struct config_set_element *entry = NULL;
struct hashmap_iter iter;
if (!cs->hash_initialized)
return;
diff --git a/merge-recursive.c b/merge-recursive.c
index 6b812d
Re: [PATCH 1/1] fetch: let --jobs= parallelize --multiple, too
"Johannes Schindelin via GitGitGadget"
writes:
> +test_expect_success 'parallel' '
> + git remote add one ./bogus1 &&
> + git remote add two ./bogus2 &&
> +
> + test_must_fail env GIT_TRACE="$PWD/trace" \
> + git fetch --jobs=2 --multiple one two 2>err &&
> + grep "2 tasks" trace &&
I think this one expects to match this in run-command.c:
trace_printf("run_processes_parallel: preparing to run up to %d tasks",
n);
> + grep "one.*128" err &&
> + grep "two.*128" err
and these expect to match this in fetch.c
strbuf_addf(out, _("could not fetch '%s' (exit code: %d)\n"),
It would have been nice to fellow contributors, if the grep patterns
were written a bit more tightly. It would allow people who debug
test failure to more easily identify which message the patterns are
trying to catch.
In any case, the latter two needs to be guarded against
gettext-poison, I would think. Without addressing the vagueness of
the pattern, at least the following needs to be squashed to help the
CI.
Thanks.
---
t/t5514-fetch-multiple.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
index cce829b989..33f5220a53 100755
--- a/t/t5514-fetch-multiple.sh
+++ b/t/t5514-fetch-multiple.sh
@@ -190,8 +190,8 @@ test_expect_success 'parallel' '
test_must_fail env GIT_TRACE="$PWD/trace" \
git fetch --jobs=2 --multiple one two 2>err &&
grep "2 tasks" trace &&
- grep "one.*128" err &&
- grep "two.*128" err
+ test_i18ngrep "one.*128" err &&
+ test_i18ngrep "two.*128" err
'
test_done
--
2.23.0-686-g3bf927a9c0
Re: [PATCH v2 0/5] t4214: cleanup and demonstrate graph bug
Denton Liu writes: > Range-diff against v1: > 1: e77af8cde5 = 1: e77af8cde5 test-lib: let test_merge() perform octopus > merges micronit: I would say s/let/allow/ if I were writing this. > 2: 4a93deb3f6 = 2: 4a93deb3f6 t4214: use test_merge > 3: c09f761185 = 3: c09f761185 t4214: generate expect in their own test cases > 4: ad6d89440b = 4: ad6d89440b t4214: explicitly list tags in log > 5: 0b84bf5417 ! 5: e58c1929bc t4214: demonstrate octopus graph coloring > failure Queued and pushed out. Thanks.
Re: [PATCH -v3 0/8] fast export/import: handle nested tags, improve incremental exports
Elijah Newren writes: > This series improves the incremental export story for fast-export and > fast-import (--export-marks and --import-marks fell a bit short), > fixes a couple small export/import bugs, and enables handling nested > tags. In particular, the nested tags handling makes it so that > fast-export and fast-import can finally handle the git.git repo. > > Changes since v2 (full range-diff below): > - Code cleanup of patch 2 suggested by René > > Elijah Newren (8): > fast-export: fix exporting a tag and nothing else > fast-import: fix handling of deleted tags > fast-import: allow tags to be identified by mark labels > fast-import: add support for new 'alias' command > fast-export: add support for --import-marks-if-exists > fast-export: allow user to request tags be marked with --mark-tags > t9350: add tests for tags of things other than a commit > fast-export: handle nested tags Thanks, both. Replaced.
Re: [PATCH v3 1/1] add -i: Show progress counter in the prompt
"Kunal Tyagi via GitGitGadget" writes: > From: Kunal Tyagi > > Report the current hunk count and total number of hunks for the current > file in the prompt > Adjust the expected output in some tests to account for new data on the prompt > > Signed-off-by: Kunal Tyagi > --- > git-add--interactive.perl | 2 +- > t/t3701-add-interactive.sh | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Thanks; queued.
