Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
Hi Junio, On Sat, 16 Sep 2017, Junio C Hamano wrote: > Johannes Schindelinwrites: > > > On Fri, 15 Sep 2017, Junio C Hamano wrote: > > > >> -- > >> [Cooking] > >> > >> [...] > >> > >> * mk/diff-delta-uint-may-be-shorter-than-ulong (2017-08-10) 1 commit > >> ... > >> Dropped, as it was rerolled for review as part of a larger series. > >> cf. <1502914591-26215-1-git-send-email-martin@mail.zuhause> > >> > >> [...] > >> > >> * mk/use-size-t-in-zlib (2017-08-10) 1 commit > >> ... > >> Dropped, as it was rerolled for review as part of a larger series. > >> cf. <1502914591-26215-1-git-send-email-martin@mail.zuhause> > >> > >> > >> * mk/diff-delta-avoid-large-offset (2017-08-11) 1 commit > >> ... > >> Dropped, as it was rerolled for review as part of a larger series. > >> cf. <1502914591-26215-1-git-send-email-martin@mail.zuhause> > >> > >> > >> -- > >> [Discarded] > >> > >> [...] > > > > These three topics are still in the wrong category. Please fix. > > Hmph, but did the larger series these refer to actually land? As I have to read these long mails to keep track of the current status of some submissions, I do not care. However, in the context of this mail, it does not make sense to have discarded patch series in the cooking section. It's simply confusing. Ciao, Dscho
Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
Sahil Duawrites: > On Fri, Sep 22, 2017 at 5:39 AM, Junio C Hamano wrote: >> >> So here is such an update. As the topic is not in 'next' yet, it >> could also be implemented by replacing patch(es) in the series, but >> doing it as a follow-up fix made it easier to see what got changed >> (both in the code and in the tests), so that is how I decided to do >> this patch. >> > Awesome! Thanks for the patch. It was easier than I'd have expected it > to be. Looks like it fixes the concerns of moving head. Is there > anythign required from my side on this features / series of patches? If you now agree with the updated behaviour and think it makes more sense that "git branch -c new HEAD" does not check out 'new', then there is nothing required. Well, other than the usual finding, reporting and optionally fixing bugs in the documentation and the code, that is ;-) Thanks.
Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
On Fri, Sep 22, 2017 at 5:39 AM, Junio C Hamanowrote: > Junio C Hamano writes: > >> My understanding of the next step was for those who are interested >> in moving this topic forward to update these patches in that >> direction. > > Well, I am one of those who are interested in moving this topic > forward, not because I'm likely to use it, but because the fewer > number of topics I have to keep in flight, the easier my work gets. > > So here is such an update. As the topic is not in 'next' yet, it > could also be implemented by replacing patch(es) in the series, but > doing it as a follow-up fix made it easier to see what got changed > (both in the code and in the tests), so that is how I decided to do > this patch. > Awesome! Thanks for the patch. It was easier than I'd have expected it to be. Looks like it fixes the concerns of moving head. Is there anythign required from my side on this features / series of patches? > -- >8 -- > Subject: [PATCH] branch: fix "copy" to never touch HEAD > > Probably because "git branch -c A B" piggybacked its implementation > on "git branch -m A B", when creating a new branch B by copying the > branch A that happens to be the current branch, it also updated HEAD > to point at the new branch. > > This does not match the usual expectation. If I were sitting on a > blue chair, and somebody comes and repaints it to red, I would > accept ending up sitting on a red chair, but if somebody creates a > new red chair, modelling it after the blue chair I am sitting on, I > do not expect to be booted off of the blue chair and ending up on > sitting on the red one. > > Let's fix this strange behaviour before it hits 'next'. Those who > want to create a new branch and switch to it can do "git checkout B" > after creating it by copying the current branch, or if that is so > useful to deserve a short-hand way to do so, perhaps extend "git > checkout -b B" to copy configurations while creating the new branch > B. A "copy" should remain to be "copy", not "copy and sometimes > checkout". > > Signed-off-by: Junio C Hamano > --- > builtin/branch.c | 9 +++-- > t/t3200-branch.sh | 10 +- > 2 files changed, 8 insertions(+), 11 deletions(-) > > diff --git a/builtin/branch.c b/builtin/branch.c > index 89f64f4123..e2e3692838 100644 > --- a/builtin/branch.c > +++ b/builtin/branch.c > @@ -506,12 +506,9 @@ static void copy_or_rename_branch(const char *oldname, > const char *newname, int > oldref.buf + 11); > } > > - if (replace_each_worktree_head_symref(oldref.buf, newref.buf, > logmsg.buf)) { > - if (copy) > - die(_("Branch copied to %s, but HEAD is not > updated!"), newname); > - else > - die(_("Branch renamed to %s, but HEAD is not > updated!"), newname); > - } > + if (!copy && > + replace_each_worktree_head_symref(oldref.buf, newref.buf, > logmsg.buf)) > + die(_("Branch renamed to %s, but HEAD is not updated!"), > newname); > > strbuf_release(); > > diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh > index 5d03ad16f6..be9b3784c6 100755 > --- a/t/t3200-branch.sh > +++ b/t/t3200-branch.sh > @@ -422,7 +422,7 @@ test_expect_success 'git branch --copy is a synonym for > -c' ' > test_cmp expect actual > ' > > -test_expect_success 'git branch -c ee ef should copy and checkout branch ef' > ' > +test_expect_success 'git branch -c ee ef should copy to create branch ef' ' > git checkout -b ee && > git reflog exists refs/heads/ee && > git config branch.ee.dummy Hello && > @@ -431,7 +431,7 @@ test_expect_success 'git branch -c ee ef should copy and > checkout branch ef' ' > git reflog exists refs/heads/ef && > test $(git config branch.ee.dummy) = Hello && > test $(git config branch.ef.dummy) = Hello && > - test $(git rev-parse --abbrev-ref HEAD) = ef > + test $(git rev-parse --abbrev-ref HEAD) = ee > ' > > test_expect_success 'git branch -c f/f g/g should work' ' > @@ -494,12 +494,12 @@ test_expect_success 'git branch -C c1 c2 should succeed > when c1 is checked out' > git checkout -b c1 && > git branch c2 && > git branch -C c1 c2 && > - test $(git rev-parse --abbrev-ref HEAD) = c2 > + test $(git rev-parse --abbrev-ref HEAD) = c1 > ' > > -test_expect_success 'git branch -C c1 c2 should add entries to > .git/logs/HEAD' ' > +test_expect_success 'git branch -C c1 c2 should never touch HEAD' ' > msg="Branch: copied refs/heads/c1 to refs/heads/c2" && > - grep "$msg$" .git/logs/HEAD > + ! grep "$msg$" .git/logs/HEAD > ' > > test_expect_success 'git branch -C master should work when master is checked > out' ' > -- > 2.14.1-907-g5aa63875cf > > > -- Regards Sahil Dua Software Developer Booking.com Connect on LinkedIn www.sahildua.com
Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
Junio C Hamanowrites: > My understanding of the next step was for those who are interested > in moving this topic forward to update these patches in that > direction. Well, I am one of those who are interested in moving this topic forward, not because I'm likely to use it, but because the fewer number of topics I have to keep in flight, the easier my work gets. So here is such an update. As the topic is not in 'next' yet, it could also be implemented by replacing patch(es) in the series, but doing it as a follow-up fix made it easier to see what got changed (both in the code and in the tests), so that is how I decided to do this patch. -- >8 -- Subject: [PATCH] branch: fix "copy" to never touch HEAD Probably because "git branch -c A B" piggybacked its implementation on "git branch -m A B", when creating a new branch B by copying the branch A that happens to be the current branch, it also updated HEAD to point at the new branch. This does not match the usual expectation. If I were sitting on a blue chair, and somebody comes and repaints it to red, I would accept ending up sitting on a red chair, but if somebody creates a new red chair, modelling it after the blue chair I am sitting on, I do not expect to be booted off of the blue chair and ending up on sitting on the red one. Let's fix this strange behaviour before it hits 'next'. Those who want to create a new branch and switch to it can do "git checkout B" after creating it by copying the current branch, or if that is so useful to deserve a short-hand way to do so, perhaps extend "git checkout -b B" to copy configurations while creating the new branch B. A "copy" should remain to be "copy", not "copy and sometimes checkout". Signed-off-by: Junio C Hamano --- builtin/branch.c | 9 +++-- t/t3200-branch.sh | 10 +- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 89f64f4123..e2e3692838 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -506,12 +506,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int oldref.buf + 11); } - if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf)) { - if (copy) - die(_("Branch copied to %s, but HEAD is not updated!"), newname); - else - die(_("Branch renamed to %s, but HEAD is not updated!"), newname); - } + if (!copy && + replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf)) + die(_("Branch renamed to %s, but HEAD is not updated!"), newname); strbuf_release(); diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index 5d03ad16f6..be9b3784c6 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -422,7 +422,7 @@ test_expect_success 'git branch --copy is a synonym for -c' ' test_cmp expect actual ' -test_expect_success 'git branch -c ee ef should copy and checkout branch ef' ' +test_expect_success 'git branch -c ee ef should copy to create branch ef' ' git checkout -b ee && git reflog exists refs/heads/ee && git config branch.ee.dummy Hello && @@ -431,7 +431,7 @@ test_expect_success 'git branch -c ee ef should copy and checkout branch ef' ' git reflog exists refs/heads/ef && test $(git config branch.ee.dummy) = Hello && test $(git config branch.ef.dummy) = Hello && - test $(git rev-parse --abbrev-ref HEAD) = ef + test $(git rev-parse --abbrev-ref HEAD) = ee ' test_expect_success 'git branch -c f/f g/g should work' ' @@ -494,12 +494,12 @@ test_expect_success 'git branch -C c1 c2 should succeed when c1 is checked out' git checkout -b c1 && git branch c2 && git branch -C c1 c2 && - test $(git rev-parse --abbrev-ref HEAD) = c2 + test $(git rev-parse --abbrev-ref HEAD) = c1 ' -test_expect_success 'git branch -C c1 c2 should add entries to .git/logs/HEAD' ' +test_expect_success 'git branch -C c1 c2 should never touch HEAD' ' msg="Branch: copied refs/heads/c1 to refs/heads/c2" && - grep "$msg$" .git/logs/HEAD + ! grep "$msg$" .git/logs/HEAD ' test_expect_success 'git branch -C master should work when master is checked out' ' -- 2.14.1-907-g5aa63875cf
Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
Sahil Duawrites: >> >> * sd/branch-copy (2017-06-18) 3 commits >> - branch: add a --copy (-c) option to go with --move (-m) >> - branch: add test for -m renaming multiple config sections >> - config: create a function to format section headers >> >> "git branch" learned "-c/-C" to create and switch to a new branch >> by copying an existing one. >> >> I personally do not think "branch --copy master backup" while on >> "master" that switches to "backup" is a good UI, and I *will* say >> "I told you so" when users complain after we merge this down to >> 'master'. > > Junio, what's up with this one? It's been a long time for this being > cooked in next. Do we have a plan/idea going forward or is it just > waiting for more feedback? Thanks for pinging. I was (in a strange way) hoping that it was just me who felt that a "copy" operation of the current branch that moves me to a new branch is a design mistake, and I was planning to start merging this down to 'next' based on that assumption, but IIRC it turned out that it wasn't just me. During the renewed discussion I somehow got an impression that the concensus was for "copy" to just copy without ever changing what HEAD says, and if an operation that switches to a new branch based on the current branch is needed, the right place to do so is to enhance/extend "checkout -b". My understanding of the next step was for those who are interested in moving this topic forward to update these patches in that direction.
Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
> > * sd/branch-copy (2017-06-18) 3 commits > - branch: add a --copy (-c) option to go with --move (-m) > - branch: add test for -m renaming multiple config sections > - config: create a function to format section headers > > "git branch" learned "-c/-C" to create and switch to a new branch > by copying an existing one. > > I personally do not think "branch --copy master backup" while on > "master" that switches to "backup" is a good UI, and I *will* say > "I told you so" when users complain after we merge this down to > 'master'. Junio, what's up with this one? It's been a long time for this being cooked in next. Do we have a plan/idea going forward or is it just waiting for more feedback?
Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
Lars Schneiderwrites: > SZEDER noticing a bug in this series that I was about to fix: > https://public-inbox.org/git/3b175d35-5b1c-43cd-a7e9-85693335b...@gmail.com/ > > I assume at this point a new patch is better than a re-roll, right? Thanks, yes indeed.
Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
Johannes Schindelinwrites: > Hi Junio, > > On Fri, 15 Sep 2017, Junio C Hamano wrote: > >> -- >> [Cooking] >> >> [...] >> >> * mk/diff-delta-uint-may-be-shorter-than-ulong (2017-08-10) 1 commit >> ... >> Dropped, as it was rerolled for review as part of a larger series. >> cf. <1502914591-26215-1-git-send-email-martin@mail.zuhause> >> >> [...] >> >> * mk/use-size-t-in-zlib (2017-08-10) 1 commit >> ... >> Dropped, as it was rerolled for review as part of a larger series. >> cf. <1502914591-26215-1-git-send-email-martin@mail.zuhause> >> >> >> * mk/diff-delta-avoid-large-offset (2017-08-11) 1 commit >> ... >> Dropped, as it was rerolled for review as part of a larger series. >> cf. <1502914591-26215-1-git-send-email-martin@mail.zuhause> >> >> >> -- >> [Discarded] >> >> [...] > > These three topics are still in the wrong category. Please fix. Hmph, but did the larger series these refer to actually land? If I recall correctly they were too invasive to get merged cleanly to 'next' and 'pu' without disrupting topics in flight and that is why it is not even listed there. The only two reasons a topic with a good goal and approach gets thrown into the Discarded bin are either because it is tentatively retracted to avoid disrupting other topics in flight (with the expectation to be redone later) or what it wants to solve is addressed by another topic and becomes unnecessary. These three attempt to do good things and I have been hoping that others can help moving them forward by e.g. reporting that they still cleanly merge and stand on their own as improvements at a smaller scale than the larger one that was attempted but was not queued, or if nobody volunteers, I was guessing that I might end up doing that myself. Before that happens, I'd prefer to keep them listed and topics kept in my tree, even if they are not part of 'pu'. On the other hand, because many topics have graduated to master recently, the larger one (possibly in the updated form---I do not recall what conflicts it had with what other topics) may be able to cook peacefully together with topics we have that are still in flight. If that is the case, then that larger topic will be queued and these three will truly become material for the Discarded bin.
Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
Johannes Schindelinwrites: > Please stop stating that you expect a reroll for rebase-i-extra when you > explicitly stated months ago that you would not take my v6. It gets a bit > annoying. I already explained to you why I skipped v6, which turned to be identical to v5 when the unnecessary rebase was undone. It does not have anything to do with expecting a v7 or later to fix other things already pointed out in the review. Having said that, I am getting sick of your constant hostile whining, and am inclined to merge it as-is to 'next' and advance it from there, if only to reduce these noises from you. Even if you are unwilling to, you do not have the sole possession and veto power over the area of the code (neither do I) to prevent further improvements, so I can expect others with good design taste (hopefully not me) to fix up the misuse of revision traversal implementation detail with follow-up patches.
Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
Hi Junio, On Fri, 15 Sep 2017, Junio C Hamano wrote: > -- > [Cooking] > > [...] > > * mk/diff-delta-uint-may-be-shorter-than-ulong (2017-08-10) 1 commit > . diff-delta: fix encoding size that would not fit in "unsigned int" > > The machinery to create xdelta used in pack files received the > sizes of the data in size_t, but lost the higher bits of them by > storing them in "unsigned int" during the computation, which is > fixed. > > Dropped, as it was rerolled for review as part of a larger series. > cf. <1502914591-26215-1-git-send-email-martin@mail.zuhause> > > [...] > > * mk/use-size-t-in-zlib (2017-08-10) 1 commit > . zlib.c: use size_t for size > > The wrapper to call into zlib followed our long tradition to use > "unsigned long" for sizes of regions in memory, which have been > updated to use "size_t". > > Dropped, as it was rerolled for review as part of a larger series. > cf. <1502914591-26215-1-git-send-email-martin@mail.zuhause> > > > * mk/diff-delta-avoid-large-offset (2017-08-11) 1 commit > . diff-delta: do not allow delta offset truncation > > The delta format used in the packfile cannot reference data at > offset larger than what can be expressed in 4-byte, but the > generator for the data failed to make sure the offset does not > overflow. This has been corrected. > > Dropped, as it was rerolled for review as part of a larger series. > cf. <1502914591-26215-1-git-send-email-martin@mail.zuhause> > > > -- > [Discarded] > > [...] These three topics are still in the wrong category. Please fix. Ciao, Dscho
Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
Hi Junio, On Fri, 15 Sep 2017, Junio C Hamano wrote: > * js/rebase-i-final (2017-07-27) 10 commits > - rebase -i: rearrange fixup/squash lines using the rebase--helper > - t3415: test fixup with wrapped oneline > - rebase -i: skip unnecessary picks using the rebase--helper > - rebase -i: check for missing commits in the rebase--helper > - t3404: relax rebase.missingCommitsCheck tests > - rebase -i: also expand/collapse the SHA-1s via the rebase--helper > - rebase -i: do not invent onelines when expanding/collapsing SHA-1s > - rebase -i: remove useless indentation > - rebase -i: generate the script via rebase--helper > - t3415: verify that an empty instructionFormat is handled as before > > The final batch to "git rebase -i" updates to move more code from > the shell script to C. > > Expecting a reroll. Please stop stating that you expect a reroll for rebase-i-extra when you explicitly stated months ago that you would not take my v6. It gets a bit annoying. Thanks, Dscho
Re: What's cooking in git.git (Sep 2017, #03; Fri, 15)
On 15 Sep 2017, at 07:58, Junio C Hamanowrote: > 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. > ... > * ls/travis-scriptify (2017-09-11) 3 commits > (merged to 'next' on 2017-09-14 at 8fa685d3b7) > + travis: dedent a few scripts that are indented overly deeply > + travis-ci: skip a branch build if equal tag is present > + travis-ci: move Travis CI code into dedicated scripts > > The scripts to drive TravisCI has been reorganized and then an > optimization to avoid spending cycles on a branch whose tip is > tagged has been implemented. > > Will merge to 'master'. SZEDER noticing a bug in this series that I was about to fix: https://public-inbox.org/git/3b175d35-5b1c-43cd-a7e9-85693335b...@gmail.com/ I assume at this point a new patch is better than a re-roll, right? Thanks, Lars
What's cooking in git.git (Sep 2017, #03; Fri, 15)
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 are at week #6 of this cycle. It seems that people had a productive week while I was away, which I am reasonably happy about ;-) Quite a many topics that have been in 'master' are now also merged to 'maint', so perhaps I should tag 2.14.2 soonish. 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 -- [New Topics] * cc/subprocess-handshake-missing-capabilities (2017-09-11) 1 commit - subprocess: loudly die when subprocess asks for an unsupported capability Finishing touches to a topic already in 'master'. Will merge to 'next'. * bw/protocol-v1 (2017-09-14) 8 commits - i5700: add interop test for protocol transition - http: tell server that the client understands v1 - connect: tell server that the client understands v1 - connect: teach client to recognize v1 server response - upload-pack, receive-pack: introduce protocol version 1 - daemon: recognize hidden request arguments - protocol: introduce protocol extention mechanisms - pkt-line: add packet_write function A new mechanism to upgrade the wire protocol in place is proposed and demonstrated that it works with the older versions of Git without harming them. * ez/doc-duplicated-words-fix (2017-09-14) 1 commit - doc: fix minor typos (extra/duplicated words) Typofix. Will merge to 'next'. * jk/write-in-full-fix (2017-09-14) 8 commits - read_pack_header: handle signed/unsigned comparison in read result - config: flip return value of store_write_*() - notes-merge: use ssize_t for write_in_full() return value - pkt-line: check write_in_full() errors against "< 0" - convert less-trivial versions of "write_in_full() != len" - avoid "write_in_full(fd, buf, len) != len" pattern - get-tar-commit-id: check write_in_full() return against 0 - config: avoid "write_in_full(fd, buf, len) < len" pattern Many codepaths did not diagnose write failures correctly when disks go full, due to their misuse of write_in_full() helper function, which have been corrected. Will merge to 'next'. * jn/per-repo-object-store-fixes (2017-09-14) 3 commits - replace-objects: evaluate replacement refs without using the object store - push, fetch: error out for submodule entries not pointing to commits - pack: make packed_git_mru global a value instead of a pointer Step #0 of a planned & larger series. Will merge to 'next'. * ks/commit-do-not-touch-cut-line (2017-09-15) 1 commit - commit-template: change a message to be more intuitive The explanation of the cut-line in the commit log editor has been slightly tweaked. Will merge to 'next'. * ks/help-alias-label (2017-09-14) 1 commit - help: change a message to be more precise "git help co" now says "co is aliased to ...", not "git co is". Will merge to 'next'. * mh/mmap-packed-refs (2017-09-14) 20 commits - packed-backend.c: rename a bunch of things and update comments - mmapped_ref_iterator: inline into `packed_ref_iterator` - ref_cache: remove support for storing peeled values - packed_ref_store: get rid of the `ref_cache` entirely - ref_store: implement `refs_peel_ref()` generically - packed_read_raw_ref(): read the reference from the mmapped buffer - packed_ref_iterator_begin(): iterate using `mmapped_ref_iterator` - read_packed_refs(): ensure that references are ordered when read - packed_ref_cache: keep the `packed-refs` file open and mmapped - mmapped_ref_iterator_advance(): no peeled value for broken refs - mmapped_ref_iterator: add iterator over a packed-refs file - packed_ref_cache: remember the file-wide peeling state - read_packed_refs(): read references with minimal copying - read_packed_refs(): make parsing of the header line more robust - read_packed_refs(): only check for a header at the top of the file - read_packed_refs(): use mmap to read the `packed-refs` file - die_unterminated_line(), die_invalid_line(): new functions - packed_ref_cache: add a backlink to the associated `packed_ref_store` - prefix_ref_iterator: break when we leave the prefix - ref_iterator: keep track of whether the iterator output is ordered (this branch uses mh/packed-ref-transactions.) Operations that do not touch (majority of) packed refs have been optimized by making accesses to packed-refs file lazy; we no longer pre-parse everything, and an access to a single ref in the packed-refs does not touch majority of irrelevant refs, either. Expecting a rework to also work on Windows. * nm/imap-send-with-curl (2017-09-15) 4 commits - imap-send: use curl by default when possible - imap_send: setup_curl: retreive credentials if not set