Re: ds/sparse-cone, was Re: What's cooking in git.git (Oct 2019, #03; Fri, 11)
Derrick Stolee writes: > On 10/15/2019 3:11 AM, Eric Wong wrote: >> Junio C Hamano wrote: >>> Eric Wong writes: >>> I just took a brief look, but that appears to leak memory. "hashmap_free(var, 1)" should be replaced with "hashmap_free_entries(var, struct foo, member)" Only "hashmap_free(var, 0)" can become "hashmap_free(var)" >>> >>> I deliberately avoided merge-time band-aid fixups on this topic and >>> ew/hashmap exactly because I was sure that I'd introduce a similar >>> bugs by doing so myself. Using evil merges can be a great way to >>> help multiple topics polished independently at the same time, but >>> when overused, can hide this kind of gotchas quite easily. >>> >>> A reroll on top of ew/hashmap would be desirable, now that topic is >>> ready for 'master'. >> >> Just to be clear, that reroll should come from Stolee (+Cc-ed), right? >> I'll be around to help answer questions, but also pretty busy >> with other stuff and I think Stolee knows this API pretty well :> > > I'm working on the re-roll, yes. I was waiting for ew/hashmap to merge > with history that included ds/include-exclude. Now the current 'master' > has both, so I can rebase and check everything carefully. v4 should > have every commit compile with the new hashmap API. Thanks, both.
Re: ds/sparse-cone, was Re: What's cooking in git.git (Oct 2019, #03; Fri, 11)
On 10/15/2019 3:11 AM, Eric Wong wrote: > Junio C Hamano wrote: >> Eric Wong writes: >> >>> I just took a brief look, but that appears to leak memory. >>> >>> "hashmap_free(var, 1)" should be replaced with >>> "hashmap_free_entries(var, struct foo, member)" >>> >>> Only "hashmap_free(var, 0)" can become "hashmap_free(var)" >> >> I deliberately avoided merge-time band-aid fixups on this topic and >> ew/hashmap exactly because I was sure that I'd introduce a similar >> bugs by doing so myself. Using evil merges can be a great way to >> help multiple topics polished independently at the same time, but >> when overused, can hide this kind of gotchas quite easily. >> >> A reroll on top of ew/hashmap would be desirable, now that topic is >> ready for 'master'. > > Just to be clear, that reroll should come from Stolee (+Cc-ed), right? > I'll be around to help answer questions, but also pretty busy > with other stuff and I think Stolee knows this API pretty well :> I'm working on the re-roll, yes. I was waiting for ew/hashmap to merge with history that included ds/include-exclude. Now the current 'master' has both, so I can rebase and check everything carefully. v4 should have every commit compile with the new hashmap API. Thanks for pointing out the bugs with the suggested fixups. I'll be careful as I adapt the new API. -Stolee
Re: ds/sparse-cone, was Re: What's cooking in git.git (Oct 2019, #03; Fri, 11)
Junio C Hamano wrote: > Eric Wong writes: > > > I just took a brief look, but that appears to leak memory. > > > > "hashmap_free(var, 1)" should be replaced with > > "hashmap_free_entries(var, struct foo, member)" > > > > Only "hashmap_free(var, 0)" can become "hashmap_free(var)" > > I deliberately avoided merge-time band-aid fixups on this topic and > ew/hashmap exactly because I was sure that I'd introduce a similar > bugs by doing so myself. Using evil merges can be a great way to > help multiple topics polished independently at the same time, but > when overused, can hide this kind of gotchas quite easily. > > A reroll on top of ew/hashmap would be desirable, now that topic is > ready for 'master'. Just to be clear, that reroll should come from Stolee (+Cc-ed), right? I'll be around to help answer questions, but also pretty busy with other stuff and I think Stolee knows this API pretty well :>
Re: ds/sparse-cone, was Re: What's cooking in git.git (Oct 2019, #03; Fri, 11)
Eric Wong writes: > I just took a brief look, but that appears to leak memory. > > "hashmap_free(var, 1)" should be replaced with > "hashmap_free_entries(var, struct foo, member)" > > Only "hashmap_free(var, 0)" can become "hashmap_free(var)" I deliberately avoided merge-time band-aid fixups on this topic and ew/hashmap exactly because I was sure that I'd introduce a similar bugs by doing so myself. Using evil merges can be a great way to help multiple topics polished independently at the same time, but when overused, can hide this kind of gotchas quite easily. A reroll on top of ew/hashmap would be desirable, now that topic is ready for 'master'. Thanks.
Re: ds/sparse-cone, was Re: What's cooking in git.git (Oct 2019, #03; Fri, 11)
Johannes Schindelin wrote:
> On Fri, 11 Oct 2019, Junio C Hamano wrote:
> > * ds/sparse-cone (2019-10-08) 17 commits
> > - sparse-checkout: cone mode should not interact with .gitignore
> > - sparse-checkout: write using lockfile
> > - sparse-checkout: update working directory in-process
> > - sparse-checkout: sanitize for nested folders
> > - read-tree: show progress by default
> > - unpack-trees: add progress to clear_ce_flags()
> > - unpack-trees: hash less in cone mode
> > - sparse-checkout: init and set in cone mode
> > - sparse-checkout: use hashmaps for cone patterns
> > - sparse-checkout: add 'cone' mode
> > - trace2: add region in clear_ce_flags
> > - sparse-checkout: create 'disable' subcommand
> > - sparse-checkout: add '--stdin' option to set subcommand
> > - sparse-checkout: 'set' subcommand
> > - clone: add --sparse mode
> > - sparse-checkout: create 'init' subcommand
> > - sparse-checkout: create builtin with 'list' subcommand
> >
> > Management of sparsely checked-out working tree has gained a
> > dedicated "sparse-checkout" command.
> >
> > Seems not to play well with the hashmap updates.
>
> Hrm. I had sent out links to the three fixups needed to make the build
> green:
>
> https://public-inbox.org/git/nycvar.qro.7.76.6.1910081055210...@tvgsbejvaqbjf.bet/
>
> In particular, the patches to squash were:
>
> https://github.com/git-for-windows/git/commit/f74259754971b427a14e6290681e18950824b99d
> https://github.com/git-for-windows/git/commit/124c8bc08e974e76ca7d956dc07eb288e71d639e
> https://github.com/git-for-windows/git/commit/45948433d1b48ff513fbd37f134c0f1491c78192
> diff --git a/dir.c b/dir.c
> index 0135f9e2180..9efcdc9aacd 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -706,8 +710,8 @@ static void add_pattern_to_hashsets(struct pattern_list
> *pl, struct path_pattern
>
> clear_hashmaps:
> warning(_("disabling cone pattern matching"));
> - hashmap_free(&pl->parent_hashmap, 1);
> - hashmap_free(&pl->recursive_hashmap, 1);
> + hashmap_free(&pl->parent_hashmap);
> + hashmap_free(&pl->recursive_hashmap);
I just took a brief look, but that appears to leak memory.
"hashmap_free(var, 1)" should be replaced with
"hashmap_free_entries(var, struct foo, member)"
Only "hashmap_free(var, 0)" can become "hashmap_free(var)"
ds/sparse-cone, was Re: What's cooking in git.git (Oct 2019, #03; Fri, 11)
Hi Junio,
On Fri, 11 Oct 2019, Junio C Hamano wrote:
> * ds/sparse-cone (2019-10-08) 17 commits
> - sparse-checkout: cone mode should not interact with .gitignore
> - sparse-checkout: write using lockfile
> - sparse-checkout: update working directory in-process
> - sparse-checkout: sanitize for nested folders
> - read-tree: show progress by default
> - unpack-trees: add progress to clear_ce_flags()
> - unpack-trees: hash less in cone mode
> - sparse-checkout: init and set in cone mode
> - sparse-checkout: use hashmaps for cone patterns
> - sparse-checkout: add 'cone' mode
> - trace2: add region in clear_ce_flags
> - sparse-checkout: create 'disable' subcommand
> - sparse-checkout: add '--stdin' option to set subcommand
> - sparse-checkout: 'set' subcommand
> - clone: add --sparse mode
> - sparse-checkout: create 'init' subcommand
> - sparse-checkout: create builtin with 'list' subcommand
>
> Management of sparsely checked-out working tree has gained a
> dedicated "sparse-checkout" command.
>
> Seems not to play well with the hashmap updates.
Hrm. I had sent out links to the three fixups needed to make the build
green:
https://public-inbox.org/git/nycvar.qro.7.76.6.1910081055210...@tvgsbejvaqbjf.bet/
In particular, the patches to squash were:
https://github.com/git-for-windows/git/commit/f74259754971b427a14e6290681e18950824b99d
https://github.com/git-for-windows/git/commit/124c8bc08e974e76ca7d956dc07eb288e71d639e
https://github.com/git-for-windows/git/commit/45948433d1b48ff513fbd37f134c0f1491c78192
For your convenience:
-- snip --
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 160afb2fd7f..fb21d6f8780 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -229,9 +229,9 @@ static void insert_recursive_pattern(struct pattern_list
*pl, struct strbuf *pat
e->patternlen = path->len;
e->pattern = strbuf_detach(path, NULL);
- hashmap_entry_init(e, memhash(e->pattern, e->patternlen));
+ hashmap_entry_init(&e->ent, memhash(e->pattern, e->patternlen));
- hashmap_add(&pl->recursive_hashmap, e);
+ hashmap_add(&pl->recursive_hashmap, &e->ent);
while (e->patternlen) {
char *slash = strrchr(e->pattern, '/');
@@ -245,24 +245,26 @@ static void insert_recursive_pattern(struct pattern_list
*pl, struct strbuf *pat
e = xmalloc(sizeof(struct pattern_entry));
e->patternlen = newlen;
e->pattern = xstrndup(oldpattern, newlen);
- hashmap_entry_init(e, memhash(e->pattern, e->patternlen));
+ hashmap_entry_init(&e->ent, memhash(e->pattern, e->patternlen));
- if (!hashmap_get(&pl->parent_hashmap, e, NULL))
- hashmap_add(&pl->parent_hashmap, e);
+ if (!hashmap_get_entry(&pl->parent_hashmap, e, ent, NULL))
+ hashmap_add(&pl->parent_hashmap, &e->ent);
}
}
static void write_cone_to_file(FILE *fp, struct pattern_list *pl)
{
int i;
- struct pattern_entry *entry;
+ struct hashmap_entry *e;
struct hashmap_iter iter;
struct string_list sl = STRING_LIST_INIT_DUP;
struct strbuf parent_pattern = STRBUF_INIT;
hashmap_iter_init(&pl->parent_hashmap, &iter);
- while ((entry = hashmap_iter_next(&iter))) {
- if (hashmap_get(&pl->recursive_hashmap, entry, NULL))
+ while ((e = hashmap_iter_next(&iter))) {
+ struct pattern_entry *entry =
+ container_of(e, struct pattern_entry, ent);
+ if (hashmap_get_entry(&pl->recursive_hashmap, entry, ent, NULL))
continue;
if (!hashmap_contains_parent(&pl->recursive_hashmap,
@@ -286,7 +288,9 @@ static void write_cone_to_file(FILE *fp, struct
pattern_list *pl)
string_list_clear(&sl, 0);
hashmap_iter_init(&pl->recursive_hashmap, &iter);
- while ((entry = hashmap_iter_next(&iter))) {
+ while ((e = hashmap_iter_next(&iter))) {
+ struct pattern_entry *entry =
+ container_of(e, struct pattern_entry, ent);
if (!hashmap_contains_parent(&pl->recursive_hashmap,
entry->pattern,
&parent_pattern))
diff --git a/dir.c b/dir.c
index 0135f9e2180..9efcdc9aacd 100644
--- a/dir.c
+++ b/dir.c
@@ -612,10 +612,13 @@ void parse_path_pattern(const char **pattern,
}
int pl_hashmap_cmp(const void *unused_cmp_data,
- const void *a, const void *b, const void *key)
+ const struct hashmap_entry *a, const struct hashmap_entry *b,
+ const void *key)
{
- const struct pattern_entry *ee1 = (const struct pattern_entry *)a;
- const struct pattern_entry *ee2 = (const struct pattern_entry *)b;
+ const struct pattern_entry *ee1 =
+ container_of
Re: What's cooking in git.git (Oct 2019, #03; Fri, 11)
Junio C Hamano writes: > By the way, I think I made a mistake in my calendar math. > > This topic was merged to 'next' on the 7th and it is not especially > tricky; unless I (or somebody else) find glaring issues in it during > the final sanity check before merging it to 'master' during the next > batch, it would take the normal course to 'master' before the 18th, > on which the rc0 is planned. Unless I lose power or net due to typhoon, I plan to examine topics that were merged to 'next' to see which ones should be part of the next batch over the weekend. This will be among those topics. Thanks.
Re: What's cooking in git.git (Oct 2019, #03; Fri, 11)
Elijah Newren writes: > Did I shoot myself in the foot by being quick to jump on Rene's couple > of cosmetic touch-up suggestions he posted over a week after the > series was originally posted? If the suggested updates were simple enough to do and would improve the result sufficiently (which is probably true in this case), it is possible that the topic would have been marked "Expecting a reroll" and stayed out of 'next' longer. Or I may have missed the suggested updates before merging the known-incomplete series to 'next', and the topic may have got marked "Expecting a follow-up" and stayed in that state until the update happened. Or I may have simply missed the suggestion and allowed you to successfully game the system. So you have 2/3 chance of delaying the topic's graduation by doing so ;-). But over time in the longer run, those contributors who prioritize quality over haste are rewarded with more trust (which results in shorter gestation period for their topics in both 'pu' and 'next'), while those contributors with tendency to sneakily gaming the system eventually get caught and their contribution has to be looked at with finer toothed comb than usual, making more work on everybody and delaying the whole thing, including but not limited to their topics. At least, that is how the system hopefully works. By the way, I think I made a mistake in my calendar math. This topic was merged to 'next' on the 7th and it is not especially tricky; unless I (or somebody else) find glaring issues in it during the final sanity check before merging it to 'master' during the next batch, it would take the normal course to 'master' before the 18th, on which the rc0 is planned.
Re: What's cooking in git.git (Oct 2019, #03; Fri, 11)
On Fri, Oct 11, 2019 at 9:42 AM Junio C Hamano wrote: > > Elijah Newren writes: > > >> * en/fast-imexport-nested-tags (2019-10-04) 8 commits > >> (merged to 'next' on 2019-10-07 at 3e75779e10) > >> + fast-export: handle nested tags > >> ... > >> + fast-export: fix exporting a tag and nothing else > >> > >> Updates to fast-import/export. > >> > >> Will merge to 'master'. > > > > Any chance this will merge down before 2.24.0? I'd really like to see > > and use it within filter-repo. > > A few general guidelines I use are that a typical topic spends 1 > week in 'next' (a trivial one-liner may be there for much shorter > time, an involved multi-patch topic that touches the core part of > the system may have to spend more), that an involved topic that is > not in 'master' by rc0 would not appear in the next release, and > that any topic that is not in 'master' by rc1 needs compelling > reason to be in the next release. So it is cutting a bit too close > for this topic, it seems, but we'll see. :-( Did I shoot myself in the foot by being quick to jump on Rene's couple of cosmetic touch-up suggestions he posted over a week after the series was originally posted? In other words, if I hadn't stopped you from merging the series down to next to incorporate those clean-ups[1], would the story have been different? [1] https://public-inbox.org/git/CABPp-BHvzyLf=wwhv45qkdkjitvwhtsmdfa0hd5ejf5fmhh...@mail.gmail.com/
Re: What's cooking in git.git (Oct 2019, #03; Fri, 11)
Elijah Newren writes: >> * en/fast-imexport-nested-tags (2019-10-04) 8 commits >> (merged to 'next' on 2019-10-07 at 3e75779e10) >> + fast-export: handle nested tags >> ... >> + fast-export: fix exporting a tag and nothing else >> >> Updates to fast-import/export. >> >> Will merge to 'master'. > > Any chance this will merge down before 2.24.0? I'd really like to see > and use it within filter-repo. A few general guidelines I use are that a typical topic spends 1 week in 'next' (a trivial one-liner may be there for much shorter time, an involved multi-patch topic that touches the core part of the system may have to spend more), that an involved topic that is not in 'master' by rc0 would not appear in the next release, and that any topic that is not in 'master' by rc1 needs compelling reason to be in the next release. So it is cutting a bit too close for this topic, it seems, but we'll see.
Re: What's cooking in git.git (Oct 2019, #03; Fri, 11)
On Fri, Oct 11, 2019 at 12:35 AM Junio C Hamano wrote: > [Cooking] [...] > * en/fast-imexport-nested-tags (2019-10-04) 8 commits > (merged to 'next' on 2019-10-07 at 3e75779e10) > + 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 'master'. Any chance this will merge down before 2.24.0? I'd really like to see and use it within filter-repo.
What's cooking in git.git (Oct 2019, #03; Fri, 11)
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. We may want to merge down fixes accumulated on the 'master' front to 'maint', to prepare for v2.23.1 soonish. I'll look into it over the weekend. 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 -- [Graduated to "master"] * ab/pcre-jit-fixes (2019-08-19) 18 commits (merged to 'next' on 2019-10-04 at 2d55f2b470) + grep: under --debug, show whether PCRE JIT is enabled + grep: do not enter PCRE2_UTF mode on fixed matching + grep: stess test PCRE v2 on invalid UTF-8 data + grep: create a "is_fixed" member in "grep_pat" + grep: consistently use "p->fixed" in compile_regexp() + grep: stop using a custom JIT stack with PCRE v1 + grep: stop "using" a custom JIT stack with PCRE v2 + grep: remove overly paranoid BUG(...) code + grep: use PCRE v2 for optimized fixed-string search + grep: remove the kwset optimization + grep: drop support for \0 in --fixed-strings + grep: make the behavior for NUL-byte in patterns sane + grep tests: move binary pattern tests into their own file + grep tests: move "grep binary" alongside the rest + grep: inline the return value of a function call used only once + t4210: skip more command-line encoding tests on MinGW + grep: don't use PCRE2?_UTF8 with "log --encoding=" + log tests: test regex backends in "--encode=" tests (this branch is used by cb/pcre1-cleanup.) A few simplification and bugfixes to PCRE interface. * ah/cleanups (2019-10-03) 4 commits (merged to 'next' on 2019-10-04 at 1345f09afb) + git_mkstemps_mode(): replace magic numbers with computed value + wrapper: use a loop instead of repetitive statements + diffcore-break: use a goto instead of a redundant if statement + commit-graph: remove a duplicate assignment Miscellaneous code clean-ups. * am/visual-studio-config-fix (2019-09-28) 1 commit (merged to 'next' on 2019-10-04 at 135d93143b) + contrib/buildsystems: fix Visual Studio Debug configuration Dev support. * as/shallow-slab-use-fix (2019-10-02) 1 commit (merged to 'next' on 2019-10-04 at f3a22d2b18) + shallow.c: don't free unallocated slabs Correct code that tried to reference all entries in a sparse array of pointers by mistake. * bc/object-id-part17 (2019-08-19) 26 commits (merged to 'next' on 2019-10-04 at b0460b0db2) + midx: switch to using the_hash_algo + builtin/show-index: replace sha1_to_hex + rerere: replace sha1_to_hex + builtin/receive-pack: replace sha1_to_hex + builtin/index-pack: replace sha1_to_hex + packfile: replace sha1_to_hex + wt-status: convert struct wt_status to object_id + cache: remove null_sha1 + builtin/worktree: switch null_sha1 to null_oid + builtin/repack: write object IDs of the proper length + pack-write: use hash_to_hex when writing checksums + sequencer: convert to use the_hash_algo + bisect: switch to using the_hash_algo + sha1-lookup: switch hard-coded constants to the_hash_algo + config: use the_hash_algo in abbrev comparison + combine-diff: replace GIT_SHA1_HEXSZ with the_hash_algo + bundle: switch to use the_hash_algo + connected: switch GIT_SHA1_HEXSZ to the_hash_algo + show-index: switch hard-coded constants to the_hash_algo + blame: remove needless comparison with GIT_SHA1_HEXSZ + builtin/rev-parse: switch to use the_hash_algo + builtin/blame: switch uses of GIT_SHA1_HEXSZ to the_hash_algo + builtin/receive-pack: switch to use the_hash_algo + fetch-pack: use parse_oid_hex + patch-id: convert to use the_hash_algo + builtin/replace: make hash size independent Preparation for SHA-256 upgrade continues. * cb/pcre1-cleanup (2019-08-26) 2 commits (merged to 'next' on 2019-10-04 at a2dd896ee8) + grep: refactor and simplify PCRE1 support + grep: make sure NO_LIBPCRE1_JIT disable JIT in PCRE1 (this branch uses ab/pcre-jit-fixes.) PCRE fixes. * dl/rev-list-doc-cleanup (2019-10-06) 1 commit (merged to 'next' on 2019-10-07 at 712594feb1) + git-rev-list.txt: prune options in synopsis Doc update. * en/clean-nested-with-ignored (2019-10-02) 13 commits (merged to 'next' on 2019-10-03 at 969ec06cc7) + dir: special case check for the possibility that pathspec is NULL (merged to 'next' on 2019-09-30 at 778cc31eac) + clean: fix theoretical path corruption + clean: rewrap overly long line + clean: avoid removing untracked files in a nested git repository + clean: disambiguate the definition of -d + git-clean.txt: do not claim we will delete files with -n/--dry-run + dir: add commentary explaining match_pathspec_item's return value + dir: if our pathspec might match files under a dir, recurse into it + dir: make th
