Re: What's cooking in git.git (Jul 2018, #02; Wed, 18)
On 07/24, Junio C Hamano wrote:
> Jonathan Tan writes:
>
> >> 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.
> >
> > What do you think about my fixes to protocol v2 tag following [1]? There
> > was some discussion about correctness vs the drop in performance, but it
> > seems to me that there is some consensus that the drop in performance is
> > OK.
> >
> > [1]
> > https://public-inbox.org/git/cover.1528234587.git.jonathanta...@google.com/
>
> Thanks for reminding. I think I was waiting for Brandon or somebody
> else to say something after [2] as the final confirmation before
> queuing it, and then the thread was forgotten ;-)
>
> Will pick it up; it seems to have some interaction with Brandon's
> 6d1700d5 ("fetch: refactor to make function args narrower",
> 2018-06-27), and I think the correct resolution is to move your
> removal of "&& !rs->nr" to do_fetch() function where that commit
> moved to.
>
> Thanks.
>
> [2] https://public-inbox.org/git/xmqqd0vwcfkr@gitster-ct.c.googlers.com/
Yeah I still don't like it from a performance perspective, but given
people rely on this functionality I've been convinced its necessary for
correctness until we make other changes.
--
Brandon Williams
Re: What's cooking in git.git (Jul 2018, #02; Wed, 18)
Jonathan Tan writes:
>> 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.
>
> What do you think about my fixes to protocol v2 tag following [1]? There
> was some discussion about correctness vs the drop in performance, but it
> seems to me that there is some consensus that the drop in performance is
> OK.
>
> [1]
> https://public-inbox.org/git/cover.1528234587.git.jonathanta...@google.com/
Thanks for reminding. I think I was waiting for Brandon or somebody
else to say something after [2] as the final confirmation before
queuing it, and then the thread was forgotten ;-)
Will pick it up; it seems to have some interaction with Brandon's
6d1700d5 ("fetch: refactor to make function args narrower",
2018-06-27), and I think the correct resolution is to move your
removal of "&& !rs->nr" to do_fetch() function where that commit
moved to.
Thanks.
[2] https://public-inbox.org/git/xmqqd0vwcfkr@gitster-ct.c.googlers.com/
Re: What's cooking in git.git (Jul 2018, #02; Wed, 18)
> 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. What do you think about my fixes to protocol v2 tag following [1]? There was some discussion about correctness vs the drop in performance, but it seems to me that there is some consensus that the drop in performance is OK. [1] https://public-inbox.org/git/cover.1528234587.git.jonathanta...@google.com/
ag/rebase-i-in-c, was Re: What's cooking in git.git (Jul 2018, #02; Wed, 18)
Hi Junio, Le 19/07/2018 à 00:03, Junio C Hamano a écrit : > * ag/rebase-i-in-c (2018-07-10) 13 commits > - rebase -i: rewrite the rest of init_revisions_and_shortrevisions in C > - rebase -i: implement the logic to initialize the variable $revision in C > - rebase--interactive: remove unused modes and functions > - rebase--interactive: rewrite complete_action() in C > - sequencer: change the way skip_unnecessary_picks() returns its result > - sequencer: refactor append_todo_help() to write its message to a buffer > - rebase -i: rewrite checkout_onto() in C > - rebase -i: rewrite setup_reflog_action() in C > - sequencer: add a new function to silence a command, except if it fails > - rebase-interactive: rewrite the edit-todo functionality in C > - editor: add a function to launch the sequence editor > - rebase--interactive: rewrite append_todo_help() in C > - sequencer: make two functions and an enum from sequencer.c public > > Piecemeal rewrite of the remaining "rebase -i" machinery in C. > > Expecting a reroll. > > The early parts of the series seem solidifying; perhaps with a > reroll or two, they become 'next' material? I am working on new changes (rewriting init_basic_state(), and making rebase--interactive a builtin), so it will probably need at least one more reroll before being trully ready for 'next'. It’s not completely finished yet, I hope to send it Monday or Tuesday. Cheers, Alban
Re: ds/multi-pack-index (was Re: What's cooking in git.git (Jul 2018, #02; Wed, 18))
On 7/20/2018 12:09 PM, Junio C Hamano wrote: Derrick Stolee writes: What's the doneness of this one? I vaguely recall that there was an objection against the concept as a whole (i.e. there is a way with less damage to gain the same object-abbrev performance); has it (and if anything else, they) been resolved in satisfactory fashion? I believe you're talking about Ævar's patch series [1] on unconditional abbreviation lengths. Yes, this is a total tangent, but what happened to that one? I did not queue because I was led to expect v2 to follow soonish [*1*]. Lookup speeds improve in a multi-pack environment. True. I recall that years ago there was a discussion, but nobody came up with patches, to do the consolidated .idx for exactly that reason (not the "abbrev" reason). That's the best I can do to sell the feature as it stands now (plus the 'fsck' integration that would follow after this series is accepted). Heh, 'fsck' intergration is not a 'feature' to sell anything, I would think. Nobody wants to run fsck for the sake of running it---it is just having one extra file that must not go corrupt _requires_ one to have a way to check its integrity and fsck is the logical place to do so X-<. Yep. I didn't mean 'fsck' is a selling point, but that it is an important thing to build for anything that is going in the objects directory. I mention it only to say that I'm committed to providing that functionality. In any case, we've had this for about a week in 'pu' after 4 iterations, and review comments seem to have quieted down [*2*], so let's consider merging it down to 'next'. I think at least I need to "commit --amend" (or something like that) 16/23. Right. There is a commit message error and some spaces to insert. See [2] if you need a reminder. Thanks! [2] https://public-inbox.org/git/xmqqin5kupu3@gitster-ct.c.googlers.com/ [Footnotes] *1* <87a7s4471y@evledraar.gmail.com> *2* That does not indicate either of these two: - nobody is interested in the topic - the topic is now without any flaw It only means that keeping it in 'pu' as a dormant topic would not do anybody any good.
Re: ds/multi-pack-index (was Re: What's cooking in git.git (Jul 2018, #02; Wed, 18))
Derrick Stolee writes: >> What's the doneness of this one? I vaguely recall that there was >> an objection against the concept as a whole (i.e. there is a way >> with less damage to gain the same object-abbrev performance); has >> it (and if anything else, they) been resolved in satisfactory >> fashion? > > I believe you're talking about Ævar's patch series [1] on > unconditional abbreviation lengths. Yes, this is a total tangent, but what happened to that one? I did not queue because I was led to expect v2 to follow soonish [*1*]. > Lookup speeds improve in a multi-pack environment. True. I recall that years ago there was a discussion, but nobody came up with patches, to do the consolidated .idx for exactly that reason (not the "abbrev" reason). > That's the best I can do to sell the feature as it stands now (plus > the 'fsck' integration that would follow after this series is > accepted). Heh, 'fsck' intergration is not a 'feature' to sell anything, I would think. Nobody wants to run fsck for the sake of running it---it is just having one extra file that must not go corrupt _requires_ one to have a way to check its integrity and fsck is the logical place to do so X-<. In any case, we've had this for about a week in 'pu' after 4 iterations, and review comments seem to have quieted down [*2*], so let's consider merging it down to 'next'. I think at least I need to "commit --amend" (or something like that) 16/23. [Footnotes] *1* <87a7s4471y@evledraar.gmail.com> *2* That does not indicate either of these two: - nobody is interested in the topic - the topic is now without any flaw It only means that keeping it in 'pu' as a dormant topic would not do anybody any good.
ds/multi-pack-index (was Re: What's cooking in git.git (Jul 2018, #02; Wed, 18))
On 7/18/2018 6:03 PM, Junio C Hamano wrote: * ds/multi-pack-index (2018-07-12) 23 commits - midx: clear midx on repack - packfile: skip loading index if in multi-pack-index - midx: prevent duplicate packfile loads - midx: use midx in approximate_object_count - midx: use existing midx when writing new one - midx: use midx in abbreviation calculations - midx: read objects from multi-pack-index - config: create core.multiPackIndex setting - midx: write object offsets - midx: write object id fanout chunk - midx: write object ids in a chunk - midx: sort and deduplicate objects from packfiles - midx: read pack names into array - multi-pack-index: write pack names in chunk - multi-pack-index: read packfile list - packfile: generalize pack directory list - t5319: expand test data - multi-pack-index: load into memory - midx: write header information to lockfile - multi-pack-index: add 'write' verb - multi-pack-index: add builtin - multi-pack-index: add format details - multi-pack-index: add design document When there are too many packfiles in a repository (which is not recommended), looking up an object in these would require consulting many pack .idx files; a new mechanism to have a single file that consolidates all of these .idx files is introduced. What's the doneness of this one? I vaguely recall that there was an objection against the concept as a whole (i.e. there is a way with less damage to gain the same object-abbrev performance); has it (and if anything else, they) been resolved in satisfactory fashion? I believe you're talking about Ævar's patch series [1] on unconditional abbreviation lengths. His patch gets similar speedups by completely eliminating the abbreviation computation in favor of a relative increase that is very likely to avoid collisions. While abbreviation speedups are the most dramatic measurable improvement by the multi-pack-index feature, it is not the only important feature. Lookup speeds improve in a multi-pack environment. While only the largest of largest repos have trouble repacking into a single pack, there are many scenarios where users disable auto-gc and do not repack frequently. On-premise build machines are the ones I know about the most: these machines are run 24/7 to perform incremental fetches against a remote and kick off a build. Admins frequently turn off GC so the build times are not impacted. Eventually, their performance does degrade due to the number of packfiles. The answer we give to them is to set up scheduled maintenance to repack. These users don't need the space savings of a repack, but just need consistent performance and high up-time. The multi-pack-index could assist here (as long as we set up auto-computing the multi-pack-index after a fetch). That's the best I can do to sell the feature as it stands now (plus the 'fsck' integration that would follow after this series is accepted). I have mentioned the potential for the multi-pack-index to do the following: * Store metadata about the packfiles, possibly replacing the .keep and .promisor files, and allowing other extensions to inform repack algorithms. * Store a stable object order, allowing the reachability bitmap to be computed at a different cadence from repacking the packfiles. I'm interested in these applications, but I will admit that they are not on the top of my priority list at the moment. Right now, I'm focused on reaching feature parity with the version of the MIDX we have in our GVFS fork of Git, and then extending the feature to have incremental multi-pack-index files to solve the "big write" problem. Thanks, -Stolee [1] https://public-inbox.org/git/20180608224136.20220-1-ava...@gmail.com/T/#u [PATCH 00/20] unconditional O(1) SHA-1 abbreviation
Re: What's cooking in git.git (Jul 2018, #02; Wed, 18)
On Wed, Jul 18, 2018 at 3:03 PM, Junio C Hamano wrote: > * en/rebase-i-microfixes (2018-06-27) 3 commits > (merged to 'next' on 2018-07-11 at d913ca0f77) > + git-rebase--merge: modernize "git-$cmd" to "git $cmd" > + Fix use of strategy options with interactive rebases > + t3418: add testcase showing problems with rebase -i and strategy options > > Will merge to 'master'. This series showed up in the "Graduated to master" section of your email and the series shows up on master; did you just forget to remove this last line? > * en/abort-df-conflict-fixes (2018-07-16) 2 commits > - read-cache: fix directory/file conflict handling in read_index_unmerged() > - t1015: demonstrate directory/file conflict recovery failures > > "git merge --abort" etc. did not clean things up properly when > there were conflicted entries in certain order that are involved > in D/F conflicts. This has been corrected. > > This may have to be rebased on an older maintenance track before > moving forward. Would you like me to rebase on maint and resubmit? Alternatively, git cherry-pick -Xours master..en/abort-df-conflict-fixes will backport the fixes to maint, with the only downside that it leaves some unnecessary (but innocuous) double 'git reset --hard' invocations in t6042. Just let me know whatever is easiest for you. > * en/t6042-insane-merge-rename-testcases (2018-07-03) 3 commits > - t6042: add testcase covering long chains of rename conflicts > - t6042: add testcase covering rename/rename(2to1)/delete/delete conflict > - t6042: add testcase covering rename/add/delete conflict type > > Various glitches in the heuristics of merge-recursive strategy have > been documented in new tests. > > Will merge to 'next'. > > I am not sure if there is a single "correct" answer everybody can > agree on for each of these "insane" cases, though. Yeah, I agree. I was a little unsure about adding the expected "correct" answer in these testcases for fear I would just end up modifying it whenever I finally implement a fix. However, it was clear that current handling for these testcases is suboptimal, and ultimately I decided it'd make sense to just take my best guess at "correct" for now and deal with any modifications later. *shrug* I'm not sure what, if any changes to make to this series because of this, though; for now, I think it's fine as-is.
Re: What's cooking in git.git (Jul 2018, #02; Wed, 18)
Eric Sunshine writes: > I guess the two fixup!'s were meant to be squashed (and it's probably > too late to do so now?). I guess so. That is what we get from trying to be nice to reduce the number of iterations and cheating by short-circuitting the process X-<.
Re: What's cooking in git.git (Jul 2018, #02; Wed, 18)
Stefan Beller writes: >> It seems to pass its own self-tests standalone, but seems to break >> horribly when merged to 'pu'. > > It actually breaks combined with sb/submodule-core-worktree :-( > as that introduces another user of $name in git-submodule.sh > which we eliminate in this series. Thanks for diagnosing it. I couldn't get around to figure out the interaction before pushing out.
Re: What's cooking in git.git (Jul 2018, #02; Wed, 18)
On Wed, Jul 18, 2018 at 6:04 PM Junio C Hamano wrote: > * es/test-lint-one-shot-export (2018-07-16) 5 commits > (merged to 'next' on 2018-07-18 at 26a6124963) > + t/check-non-portable-shell: detect "FOO=bar shell_func" > + t/check-non-portable-shell: make error messages more compact > + t/check-non-portable-shell: stop being so polite > + t6046/t9833: fix use of "VAR=VAL cmd" with a shell function > + Merge branch 'jc/t3404-one-shot-export-fix' into > es/test-lint-one-shot-export > (this branch uses jc/t3404-one-shot-export-fix.) > > Look for broken use of "vAR=VAL shell_func" in test scripts as part > of test-lint. s/vAR/VAR/ > * bb/pedantic (2018-07-09) 8 commits > (merged to 'next' on 2018-07-18 at e9d075e8ed) > + utf8.c: avoid char overflow > + string-list.c: avoid conversion from void * to function pointer > + sequencer.c: avoid empty statements at top level > + convert.c: replace "\e" escapes with "\033". > + fixup! refs/refs-internal.h: avoid forward declaration of an enum > + refs/refs-internal.h: avoid forward declaration of an enum > + fixup! connect.h: avoid forward declaration of an enum > + connect.h: avoid forward declaration of an enum I guess the two fixup!'s were meant to be squashed (and it's probably too late to do so now?).
Re: What's cooking in git.git (Jul 2018, #02; Wed, 18)
> * ot/ref-filter-object-info (2018-07-17) 5 commits > - ref-filter: use oid_object_info() to get object > - ref-filter: merge get_obj and get_object > - ref-filter: initialize eaten variable > - ref-filter: fill empty fields with empty values > - ref-filter: add info_source to valid_atom > > A few atoms like %(objecttype) and %(objectsize) in the format > specifier of "for-each-ref --format=" can be filled without > getting the full contents of the object, but just with the object > header. These cases have been optimzied by calling > oid_object_info() API. > > What's the doneness of this one? > I fixed all known issues, I hope we can move forward with it. Thank you!
Re: What's cooking in git.git (Jul 2018, #02; Wed, 18)
On Wed, Jul 18, 2018 at 3:04 PM Junio C Hamano wrote: > Many topics have moved to 'master' and 'next' from 'next' to 'pu' > respectively. As I needed to re-resolve semantic merge conflicts > while reordering some topics (i.e. some that were merged earlier in > 'pu' are left in 'pu' while another topic that had interactions with > them and required evil merge to resolve semantic conflicts > leapfrogged to 'next'---the semantic conflict resolution then need > to happen at a different merge than done earlier in 'pu' while > rebuilding today's integration), I didn't have enough time to spend > on new topics posted to the list in the past few days. > > * sb/submodule-update-in-c (2018-07-17) 6 commits > - submodule--helper: introduce new update-module-mode helper > - builtin/submodule--helper: factor out method to update a single submodule > - builtin/submodule--helper: store update_clone information in a struct > - builtin/submodule--helper: factor out submodule updating > - git-submodule.sh: rename unused variables > - git-submodule.sh: align error reporting for update mode to use path > > "git submodule update" is getting rewritten piece-by-piece into C. > > It seems to pass its own self-tests standalone, but seems to break > horribly when merged to 'pu'. It actually breaks combined with sb/submodule-core-worktree :-( as that introduces another user of $name in git-submodule.sh which we eliminate in this series. I'll build on top of that and send a fixed version. Sorry about being confused and breaking so much lately, Stefan
What's cooking in git.git (Jul 2018, #02; Wed, 18)
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. Many topics have moved to 'master' and 'next' from 'next' to 'pu' respectively. As I needed to re-resolve semantic merge conflicts while reordering some topics (i.e. some that were merged earlier in 'pu' are left in 'pu' while another topic that had interactions with them and required evil merge to resolve semantic conflicts leapfrogged to 'next'---the semantic conflict resolution then need to happen at a different merge than done earlier in 'pu' while rebuilding today's integration), I didn't have enough time to spend on new topics posted to the list in the past few days. 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"] * ao/config-from-gitmodules (2018-06-26) 6 commits (merged to 'next' on 2018-07-11 at 5d88dc6fb7) + submodule-config: reuse config_from_gitmodules in repo_read_gitmodules + submodule-config: pass repository as argument to config_from_gitmodules + submodule-config: make 'config_from_gitmodules' private + submodule-config: add helper to get 'update-clone' config from .gitmodules + submodule-config: add helper function to get 'fetch' config from .gitmodules + config: move config_from_gitmodules to submodule-config.c Tighten the API to make it harder to misuse in-tree .gitmodules file, even though it shares the same syntax with configuration files, to read random configuration items from it. * bw/config-refer-to-gitsubmodules-doc (2018-06-21) 1 commit (merged to 'next' on 2018-06-29 at da6f49d292) + docs: link to gitsubmodules Docfix. * bw/protocol-v2 (2018-06-22) 1 commit (merged to 'next' on 2018-06-29 at 78090cc343) + protocol-v2 doc: put HTTP headers after request Doc fix. * dj/runtime-prefix (2018-06-26) 1 commit (merged to 'next' on 2018-07-11 at 27d99fef94) + Makefile: tweak sed invocation POSIX portability fix in Makefile to fix a glitch introduced a few releases ago. * ds/commit-graph (2018-06-28) 1 commit (merged to 'next' on 2018-07-11 at d579f733ed) + commit-graph: fix documentation inconsistencies Docfix. * ds/ewah-cleanup (2018-06-21) 10 commits (merged to 'next' on 2018-06-28 at 9cd7c0d54a) + ewah: delete unused 'rlwit_discharge_empty()' + ewah: drop ewah_serialize_native function + ewah: drop ewah_deserialize function + ewah_io: delete unused 'ewah_serialize()' + ewah_bitmap: delete unused 'ewah_or()' + ewah_bitmap: delete unused 'ewah_not()' + ewah_bitmap: delete unused 'ewah_and_not()' + ewah_bitmap: delete unused 'ewah_and()' + ewah/bitmap.c: delete unused 'bitmap_each_bit()' + ewah/bitmap.c: delete unused 'bitmap_clear()' Originally merged to 'next' on 2018-06-22 Remove unused function definitions and declarations from ewah bitmap subsystem. * en/merge-recursive-cleanup (2018-06-12) 6 commits (merged to 'next' on 2018-06-28 at 1a3646eb7d) + merge-recursive: add pointer about unduly complex looking code + merge-recursive: rename conflict_rename_*() family of functions + merge-recursive: clarify the rename_dir/RENAME_DIR meaning + merge-recursive: align labels with their respective code blocks + merge-recursive: fix numerous argument alignment issues + merge-recursive: fix miscellaneous grammar error in comment Originally merged to 'next' on 2018-06-19 Code cleanup. * en/rebase-i-microfixes (2018-06-27) 3 commits (merged to 'next' on 2018-07-11 at d913ca0f77) + git-rebase--merge: modernize "git-$cmd" to "git $cmd" + Fix use of strategy options with interactive rebases + t3418: add testcase showing problems with rebase -i and strategy options Will merge to 'master'. * jk/branch-l-0-deprecation (2018-06-22) 3 commits (merged to 'next' on 2018-06-29 at fac676dfb9) + branch: deprecate "-l" option + t: switch "branch -l" to "branch --create-reflog" + t3200: unset core.logallrefupdates when testing reflog creation (this branch is used by jk/branch-l-1-repurpose.) The "-l" option in "git branch -l" is an unfortunate short-hand for "--create-reflog", but many users, both old and new, somehow expect it to be something else, perhaps "--list". This step warns when "-l" is used as a short-hand for "--create-reflog" and warns about the future repurposing of the it when it is used. * js/enhanced-version-info (2018-06-29) 1 commit (merged to 'next' on 2018-07-11 at 815b2ea2bc) + Makefile: fix the "built from commit" code Build fix. * js/rebase-recreate-merge (2018-06-27) 1 commit (merged to 'next' on 2018-07-11 at eb8f33aaef) + rebase: fix documentation formatting Docfix. * jt/remove-pack-bitmap-global (2018-06-21) 2 commits
