All of lore.kernel.org
 help / color / mirror / Atom feed
* What's cooking in git.git (Jul 2017, #03; Mon, 10)
@ 2017-07-10 22:48 Junio C Hamano
  2017-07-11  8:28 ` Lars Schneider
  2017-07-13 12:44 ` Johannes Schindelin
  0 siblings, 2 replies; 17+ messages in thread
From: Junio C Hamano @ 2017-07-10 22:48 UTC (permalink / raw)
  To: git

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 also have tons of small updates in preparation for 2.13.3 on
'maint'.  All of them are topics that have been merged to 'master'
more than a few days ago, but I do not know how well they are tested
in the field by people using 'master' in their everyday workflow.
Ideally, our release process wants to see more people using 'next'
in their everyday workflow to keep 'master' more stable than any
tagged release, but I do not have a good idea on how to encourage
it more than we currently do.

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/sha1dc (2017-07-03) 2 commits
  (merged to 'next' on 2017-07-06 at 5a783032b7)
 + sha1collisiondetection: automatically enable when submodule is populated
 + sha1dc: optionally use sha1collisiondetection as a submodule

 The "collission-detecting" implementation of SHA-1 hash we borrowed
 from is replaced by directly binding the upstream project as our
 submodule.  Glitches on minority platforms are still being worked out.


* ab/wildmatch (2017-06-23) 1 commit
  (merged to 'next' on 2017-07-07 at 34482a9a4f)
 + wildmatch: remove unused wildopts parameter

 Minor code cleanup.


* bb/unicode-10.0 (2017-07-07) 1 commit
  (merged to 'next' on 2017-07-07 at a9c46e097b)
 + unicode: update the width tables to Unicode 10

 Update the character width tables.


* jk/reflog-walk-maint (2017-07-07) 4 commits
  (merged to 'next' on 2017-07-07 at 611554ba2f)
 + reflog-walk: include all fields when freeing complete_reflogs
 + reflog-walk: don't free reflogs added to cache
 + reflog-walk: duplicate strings in complete_reflogs list
  (merged to 'next' on 2017-07-06 at 7408dd80a1)
 + reflog-walk: skip over double-null oid due to HEAD rename
 (this branch is used by jk/reflog-walk.)

 After "git branch --move" of the currently checked out branch, the
 code to walk the reflog of HEAD via "log -g" and friends
 incorrectly stopped at the reflog entry that records the renaming
 of the branch.


* ks/commit-assuming-only-warning-removal (2017-06-30) 2 commits
  (merged to 'next' on 2017-07-05 at 462a72df95)
 + commit-template: distinguish status information unconditionally
 + commit-template: remove outdated notice about explicit paths

 An old message shown in the commit log template was removed, as it
 has outlived its usefulness.


* ks/typofix-commit-c-comment (2017-07-06) 1 commit
  (merged to 'next' on 2017-07-07 at 64e98fc832)
 + builtin/commit.c: fix a typo in the comment

 Typofix.


* pw/unquote-path-in-git-pm (2017-06-30) 4 commits
  (merged to 'next' on 2017-07-05 at 538ab4d599)
 + t9700: add tests for Git::unquote_path()
 + Git::unquote_path(): throw an exception on bad path
 + Git::unquote_path(): handle '\a'
 + add -i: move unquote_path() to Git.pm

 Code refactoring.


* rs/free-and-null (2017-06-29) 1 commit
  (merged to 'next' on 2017-07-06 at 9c9e1d59a2)
 + coccinelle: polish FREE_AND_NULL rules

 Code cleanup.

--------------------------------------------------
[New Topics]

* kn/ref-filter-branch-list (2017-07-10) 4 commits
  (merged to 'next' on 2017-07-10 at 35fd25c0dd)
 + ref-filter.c: drop return from void function
 + branch: set remote color in ref-filter branch immediately
 + branch: use BRANCH_COLOR_LOCAL in ref-filter format
 + branch: only perform HEAD check for local branches

 The rewrite of "git branch --list" using for-each-ref's internals
 that happened in v2.13 regressed its handling of color.branch.local;
 this has been fixed.

 Will merge to 'master'.


* rs/apply-avoid-over-reading (2017-07-09) 1 commit
  (merged to 'next' on 2017-07-10 at 2d8191ec3f)
 + apply: use strcmp(3) for comparing strings in gitdiff_verify_name()

 Code cleanup.

 Will merge to 'master'.


* rs/progress-overall-throughput-at-the-end (2017-07-09) 1 commit
 - progress: show overall rate in last update

 The progress meter did not give a useful output when we haven't had
 0.5 seconds to measure the throughput during the interval.  Instead
 show the overall throughput rate at the end, which is a much more
 useful number.

 Will merge to 'next'.


* rs/sha1-file-micro-optim (2017-07-09) 2 commits
 - SQUASH???
 - sha1_file: add slash once in for_each_file_in_obj_subdir()

 Code cleanup.

 Perhaps drop.
 cf. <f59c8256-716b-9305-2a4f-d4fe49f666ff@web.de>


* rs/urlmatch-cleanup (2017-07-09) 1 commit
  (merged to 'next' on 2017-07-10 at 2dd3e7cab0)
 + urlmatch: use hex2chr() in append_normalized_escapes()

 Code cleanup.

 Will merge to 'master'.


* rs/use-div-round-up (2017-07-10) 1 commit
  (merged to 'next' on 2017-07-10 at accb7919da)
 + use DIV_ROUND_UP

 Code cleanup.

 Will merge to 'master'.


* rs/wt-status-cleanup (2017-07-10) 1 commit
  (merged to 'next' on 2017-07-10 at d8939f683a)
 + wt-status: use separate variable for result of shorten_unambiguous_ref

 Code cleanup.

 Will merge to 'master'.


* ks/prepare-commit-msg-sample (2017-07-10) 3 commits
 - hook: add sign-off using "interpret-trailers"
 - hook: name the positional variables
 - hook: cleanup script

 Remove an example that is now obsolete from a sample hook,
 and improve an old example in it that added a sign-off manually
 to use the interpret-trailers command.


* jk/build-with-asan (2017-07-10) 5 commits
  (merged to 'next' on 2017-07-10 at 49757e1119)
 + Makefile: disable unaligned loads with UBSan
 + Makefile: turn off -fomit-frame-pointer with sanitizers
 + Makefile: add helper for compiling with -fsanitize
 + test-lib: turn on ASan abort_on_error by default
 + test-lib: set ASAN_OPTIONS variable before we run git

 The build procedure has been improved to allow building and testing
 Git with address sanitizer more easily.

 Will merge to 'master'.


* ks/fix-rebase-doc-picture (2017-07-10) 1 commit
  (merged to 'next' on 2017-07-10 at 3acb856b17)
 + doc: correct a mistake in an illustration

 Doc update.

 Will merge to 'master'.

--------------------------------------------------
[Stalled]

* mg/status-in-progress-info (2017-05-10) 2 commits
 - status --short --inprogress: spell it as --in-progress
 - status: show in-progress info for short status

 "git status" learns an option to report various operations
 (e.g. "merging") that the user is in the middle of.

 cf. <xmqqmvakcdqw.fsf@gitster.mtv.corp.google.com>


* nd/worktree-move (2017-04-20) 6 commits
 - worktree remove: new command
 - worktree move: refuse to move worktrees with submodules
 - worktree move: accept destination as directory
 - worktree move: new command
 - worktree.c: add update_worktree_location()
 - worktree.c: add validate_worktree()

 "git worktree" learned move and remove subcommands.

 Expecting a reroll.
 cf. <20170420101024.7593-1-pclouds@gmail.com>
 cf. <20170421145916.mknekgqzhxffu7di@sigill.intra.peff.net>
 cf. <d0e81b1e-5869-299e-f462-4d43dc997bd1@ramsayjones.plus.com>


* sg/clone-refspec-from-command-line-config (2017-06-16) 2 commits
 - Documentation/clone: document ignored configuration variables
 - clone: respect additional configured fetch refspecs during initial fetch
 (this branch is used by sg/remote-no-string-refspecs.)

 "git clone -c var=val" is a way to set configuration variables in
 the resulting repository, but it is more useful to also make these
 variables take effect while the initial clone is happening,
 e.g. these configuration variables could be fetch refspecs.

 Waiting for a response.
 cf. <20170617112228.vugswym4o4owf6wj@sigill.intra.peff.net>
 cf. <xmqqmv8zhdap.fsf@gitster.mtv.corp.google.com>


* js/rebase-i-final (2017-06-15) 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.
 This is at its v5.
 cf. <cover.1497444257.git.johannes.schindelin@gmx.de>

--------------------------------------------------
[Cooking]

* jc/allow-lazy-cas (2017-07-06) 1 commit
 - push: disable lazy --force-with-lease by default

 Because "git push --force-with-lease[=<ref>]" that relies on the
 stability of remote-tracking branches is unsafe when something
 fetches into the repository behind user's back, it is now disabled
 by default.  A new configuration variable can be used to enable it
 by users who know what they are doing.  This would pave the way to
 possibly turn `--force` into `--force-with-lease`.

 Will wait for feedback, then merge to and cook in 'next'.


* bc/object-id (2017-07-04) 12 commits
 - sha1_name: convert GET_SHA1* flags to GET_OID*
 - sha1_name: convert get_sha1* to get_oid*
 - Convert remaining callers of get_sha1 to get_oid.
 - builtin/verify-tag: convert to struct object_id
 - builtin/unpack-file: convert to struct object_id
 - bisect: convert bisect_checkout to struct object_id
 - builtin/update_ref: convert to struct object_id
 - sequencer: convert to struct object_id
 - remote: convert struct push_cas to struct object_id
 - submodule: convert submodule config lookup to use object_id
 - builtin/merge-tree: convert remaining caller of get_sha1 to object_id
 - builtin/fsck: convert remaining caller of get_sha1 to object_id

 Conversion from uchar[20] to struct object_id continues.

 Expecting a reroll.
 cf. <20170707212201.ofdgjaips2tw3koy@genre.crustytoothpaste.net>


* jk/reflog-walk (2017-07-09) 9 commits
  (merged to 'next' on 2017-07-09 at 7449e964c6)
 + reflog-walk: apply --since/--until to reflog dates
 + reflog-walk: stop using fake parents
 + rev-list: check reflog_info before showing usage
 + get_revision_1(): replace do-while with an early return
 + log: do not free parents when walking reflog
 + log: clarify comment about reflog cycles
 + revision: disallow reflog walking with revs->limited
 + t1414: document some reflog-walk oddities
 + Merge branch 'jk/reflog-walk-maint' into jk/reflog-walk

 Numerous bugs in walking of reflogs via "log -g" and friends have
 been fixed.

 Will cook in 'next'.


* sb/hashmap-cleanup (2017-07-05) 10 commits
 - t/helper/test-hashmap: use custom data instead of duplicate cmp functions
 - name-hash.c: drop hashmap_cmp_fn cast
 - submodule-config.c: drop hashmap_cmp_fn cast
 - remote.c: drop hashmap_cmp_fn cast
 - patch-ids.c: drop hashmap_cmp_fn cast
 - convert/sub-process: drop cast to hashmap_cmp_fn
 - config.c: drop hashmap_cmp_fn cast
 - builtin/describe: drop hashmap_cmp_fn cast
 - builtin/difftool.c: drop hashmap_cmp_fn cast
 - attr.c: drop hashmap_cmp_fn cast
 (this branch uses sb/hashmap-customize-comparison; is tangled with sb/diff-color-move.)

 Many uses of comparision callback function the hashmap API uses
 cast the callback function type when registering it to
 hashmap_init(), which defeats the compile time type checking when
 the callback interface changes (e.g. gaining more parameters).
 The callback implementations have been updated to take "void *"
 pointers and cast them to the type they expect instead.

 Will merge to 'next'.


* tb/push-to-cygwin-unc-path (2017-07-05) 1 commit
 - cygwin: allow pushing to UNC paths

 On Cygwin, similar to Windows, "git push //server/share/repository"
 ought to mean a repository on a network share that can be accessed
 locally, but this did not work correctly due to stripping the double
 slashes at the beginning.

 This may need to be heavily tested before it gets unleashed to the
 wild, as the change is at a fairly low-level code and would affect
 not just the code to decide if the push destination is local.  There
 may be unexpected fallouts in the path normalization.

 Will merge to 'next'.


* ab/grep-lose-opt-regflags (2017-06-30) 6 commits
  (merged to 'next' on 2017-07-05 at 375c0b92ea)
 + grep: remove redundant REG_NEWLINE when compiling fixed regex
 + grep: remove regflags from the public grep_opt API
 + grep: remove redundant and verbose re-assignments to 0
 + grep: remove redundant "fixed" field re-assignment to 0
 + grep: adjust a redundant grep pattern type assignment
 + grep: remove redundant double assignment to 0

 Code cleanup.

 Will merge to 'master'.


* sb/hashmap-customize-comparison (2017-06-30) 3 commits
  (merged to 'next' on 2017-07-06 at cc420805f3)
 + hashmap: migrate documentation from Documentation/technical into header
 + patch-ids.c: use hashmap correctly
 + hashmap.h: compare function has access to a data field
 (this branch is used by sb/diff-color-move and sb/hashmap-cleanup.)

 Update the hashmap API so that data to customize the behaviour of
 the comparison function can be specified at the time a hashmap is
 initialized.

 Will merge to 'master'.


* mt/p4-parse-G-output (2017-07-05) 1 commit
 . git-p4: parse marshal output "p4 -G" in p4 changes

 Use "p4 -G" to make "p4 changes" output more Python-friendly
 to parse.

 Needs review/ack from git-p4 folks.
 It still seems to break when merged to 'pu'.


* ex/deprecate-empty-pathspec-as-match-all (2017-06-23) 2 commits
  (merged to 'next' on 2017-06-26 at d026281517)
 + pathspec: die on empty strings as pathspec
 + t0027: do not use an empty string as a pathspec element

 The final step to make an empty string as a pathspec element
 illegal.  We started this by first deprecating and warning a
 pathspec that has such an element in 2.11 (Nov 2016).

 Hopefully we can merge this down to the 'master' by the end of the
 year?  A deprecation warning period that is about 1 year does not
 sound too bad.

 Will cook in 'next'.


* sb/pull-rebase-submodule (2017-06-27) 4 commits
  (merged to 'next' on 2017-07-09 at 48d2c3a51c)
 + builtin/fetch cleanup: always set default value for submodule recursing
 + pull: optionally rebase submodules (remote submodule changes only)
 + builtin/fetch: parse recurse-submodules-default at default options parsing
 + builtin/fetch: factor submodule recurse parsing out to submodule config

 "git pull --rebase --recurse-submodules" learns to rebase the
 branch in the submodules to an updated base.

 Will merge to 'master'.


* mh/packed-ref-store (2017-07-03) 30 commits
  (merged to 'next' on 2017-07-05 at 6c68c603cc)
 + read_packed_refs(): die if `packed-refs` contains bogus data
 + t3210: add some tests of bogus packed-refs file contents
 + repack_without_refs(): don't lock or unlock the packed refs
 + commit_packed_refs(): remove call to `packed_refs_unlock()`
 + clear_packed_ref_cache(): don't protest if the lock is held
 + packed_refs_unlock(), packed_refs_is_locked(): new functions
 + packed_refs_lock(): report errors via a `struct strbuf *err`
 + packed_refs_lock(): function renamed from lock_packed_refs()
 + commit_packed_refs(): use a staging file separate from the lockfile
 + commit_packed_refs(): report errors rather than dying
 + packed_ref_store: make class into a subclass of `ref_store`
 + packed-backend: new module for handling packed references
 + packed_read_raw_ref(): new function, replacing `resolve_packed_ref()`
 + packed_ref_store: support iteration
 + packed_peel_ref(): new function, extracted from `files_peel_ref()`
 + repack_without_refs(): take a `packed_ref_store *` parameter
 + get_packed_ref(): take a `packed_ref_store *` parameter
 + rollback_packed_refs(): take a `packed_ref_store *` parameter
 + commit_packed_refs(): take a `packed_ref_store *` parameter
 + lock_packed_refs(): take a `packed_ref_store *` parameter
 + add_packed_ref(): take a `packed_ref_store *` parameter
 + get_packed_refs(): take a `packed_ref_store *` parameter
 + get_packed_ref_cache(): take a `packed_ref_store *` parameter
 + validate_packed_ref_cache(): take a `packed_ref_store *` parameter
 + clear_packed_ref_cache(): take a `packed_ref_store *` parameter
 + packed_ref_store: move `packed_refs_lock` member here
 + packed_ref_store: move `packed_refs_path` here
 + packed_ref_store: new struct
 + add_packed_ref(): teach function to overwrite existing refs
 + t1408: add a test of stale packed refs covered by loose refs

 The "ref-store" code reorganization continues.

 Will merge to 'master'.


* sb/submodule-doc (2017-06-22) 1 commit
  (merged to 'next' on 2017-07-09 at fda0ceec31)
 + submodules: overhaul documentation

 Doc update.

 Will merge to 'master'.


* 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.

 Undecided.

 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
 'next' and eventually to 'master'.


* ls/filter-process-delayed (2017-06-30) 7 commits
  (merged to 'next' on 2017-07-05 at a35e644082)
 + convert: add "status=delayed" to filter process protocol
 + convert: refactor capabilities negotiation
 + convert: move multiple file filter error handling to separate function
 + convert: put the flags field before the flag itself for consistent style
 + t0021: write "OUT <size>" only on success
 + t0021: make debug log file name configurable
 + t0021: keep filter log files on comparison

 The filter-process interface learned to allow a process with long
 latency give a "delayed" response.

 Will merge to 'master'.


* bp/fsmonitor (2017-06-12) 6 commits
 - fsmonitor: add a sample query-fsmonitor hook script for Watchman
 - fsmonitor: add documentation for the fsmonitor extension.
 - fsmonitor: add test cases for fsmonitor extension
 - fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files.
 - dir: make lookup_untracked() available outside of dir.c
 - bswap: add 64 bit endianness helper get_be64

 We learned to talk to watchman to speed up "git status".

 Expecting a reroll.
 cf. <bade1166-e646-b05a-f65b-adb8da8ba0a7@gmail.com>


* sb/diff-color-move (2017-06-30) 26 commits
  (merged to 'next' on 2017-07-06 at 758ed40e4f)
 + diff: document the new --color-moved setting
 + diff.c: add dimming to moved line detection
 + diff.c: color moved lines differently, plain mode
 + diff.c: color moved lines differently
 + diff.c: buffer all output if asked to
 + diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY
 + diff.c: emit_diff_symbol learns about DIFF_SYMBOL_STAT_SEP
 + diff.c: convert word diffing to use emit_diff_symbol
 + diff.c: convert show_stats to use emit_diff_symbol
 + diff.c: convert emit_binary_diff_body to use emit_diff_symbol
 + submodule.c: migrate diff output to use emit_diff_symbol
 + diff.c: emit_diff_symbol learns DIFF_SYMBOL_REWRITE_DIFF
 + diff.c: emit_diff_symbol learns about DIFF_SYMBOL_BINARY_FILES
 + diff.c: emit_diff_symbol learns DIFF_SYMBOL_HEADER
 + diff.c: emit_diff_symbol learns DIFF_SYMBOL_FILEPAIR_{PLUS, MINUS}
 + diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_INCOMPLETE
 + diff.c: emit_diff_symbol learns DIFF_SYMBOL_WORDS[_PORCELAIN]
 + diff.c: migrate emit_line_checked to use emit_diff_symbol
 + diff.c: emit_diff_symbol learns DIFF_SYMBOL_NO_LF_EOF
 + diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_FRAGINFO
 + diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_MARKER
 + diff.c: introduce emit_diff_symbol
 + diff.c: factor out diff_flush_patch_all_file_pairs
 + diff.c: move line ending check into emit_hunk_header
 + diff.c: readability fix
 + Merge branch 'sb/hashmap-customize-comparison' into sb/diff-color-move
 (this branch uses sb/hashmap-customize-comparison; is tangled with sb/hashmap-cleanup.)

 "git diff" has been taught to optionally paint new lines that are
 the same as deleted lines elsewhere differently from genuinely new
 lines.

 Will merge to 'master'.

--------------------------------------------------
[Discarded]

* mh/packed-ref-store-prep-extra (2017-06-18) 1 commit
 . prefix_ref_iterator_advance(): relax the check of trim length

 Split out of mh/packed-ref-store-prep.


* nd/prune-in-worktree (2017-04-24) 12 commits
 . rev-list: expose and document --single-worktree
 . revision.c: --reflog add HEAD reflog from all worktrees
 . files-backend: make reflog iterator go through per-worktree reflog
 . revision.c: --all adds HEAD from all worktrees
 . refs: remove dead for_each_*_submodule()
 . revision.c: use refs_for_each*() instead of for_each_*_submodule()
 . refs: add refs_head_ref()
 . refs: move submodule slash stripping code to get_submodule_ref_store
 . refs.c: refactor get_submodule_ref_store(), share common free block
 . revision.c: --indexed-objects add objects from all worktrees
 . revision.c: refactor add_index_objects_to_pending()
 . revision.h: new flag in struct rev_info wrt. worktree-related refs

 "git gc" and friends when multiple worktrees are used off of a
 single repository did not consider the index and per-worktree refs
 of other worktrees as the root for reachability traversal, making
 objects that are in use only in other worktrees to be subject to
 garbage collection.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-10 22:48 What's cooking in git.git (Jul 2017, #03; Mon, 10) Junio C Hamano
@ 2017-07-11  8:28 ` Lars Schneider
  2017-07-11  9:11   ` Jeff King
  2017-07-13 12:44 ` Johannes Schindelin
  1 sibling, 1 reply; 17+ messages in thread
From: Lars Schneider @ 2017-07-11  8:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git


> On 11 Jul 2017, at 00:48, Junio C Hamano <gitster@pobox.com> wrote:
> 
> * ls/filter-process-delayed (2017-06-30) 7 commits
>  (merged to 'next' on 2017-07-05 at a35e644082)
> + convert: add "status=delayed" to filter process protocol
> + convert: refactor capabilities negotiation
> + convert: move multiple file filter error handling to separate function
> + convert: put the flags field before the flag itself for consistent style
> + t0021: write "OUT <size>" only on success
> + t0021: make debug log file name configurable
> + t0021: keep filter log files on comparison
> 
> The filter-process interface learned to allow a process with long
> latency give a "delayed" response.
> 
> Will merge to 'master'.

Hi Junio,

can you already tell if this topic has still a chance to be part of 2.14?

Thanks,
Lars

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-11  8:28 ` Lars Schneider
@ 2017-07-11  9:11   ` Jeff King
  2017-07-11  9:18     ` Lars Schneider
  2017-07-11 15:50     ` Junio C Hamano
  0 siblings, 2 replies; 17+ messages in thread
From: Jeff King @ 2017-07-11  9:11 UTC (permalink / raw)
  To: Lars Schneider; +Cc: Junio C Hamano, git

On Tue, Jul 11, 2017 at 10:28:08AM +0200, Lars Schneider wrote:

> > On 11 Jul 2017, at 00:48, Junio C Hamano <gitster@pobox.com> wrote:
> > 
> > * ls/filter-process-delayed (2017-06-30) 7 commits
> >  (merged to 'next' on 2017-07-05 at a35e644082)
> > + convert: add "status=delayed" to filter process protocol
> > + convert: refactor capabilities negotiation
> > + convert: move multiple file filter error handling to separate function
> > + convert: put the flags field before the flag itself for consistent style
> > + t0021: write "OUT <size>" only on success
> > + t0021: make debug log file name configurable
> > + t0021: keep filter log files on comparison
> > 
> > The filter-process interface learned to allow a process with long
> > latency give a "delayed" response.
> > 
> > Will merge to 'master'.
> 
> Hi Junio,
> 
> can you already tell if this topic has still a chance to be part of 2.14?

I'm not Junio, but we should be able to reason it out. :)

It's marked as "will merge to master", which typically means it will
happen during the next integration cycle (i.e., within a few days when
the next "What's cooking" is written).

Since 2.14 will be cut from the tip of master in a few weeks, once it's
merged it will definitely be in 2.14 (barring a revert, of course).

This holds true during release freeze, too. Anything that makes it to
master is part of the release. The stopping point there is that things
stop getting marked as "will merge to master".

-Peff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-11  9:11   ` Jeff King
@ 2017-07-11  9:18     ` Lars Schneider
  2017-07-11  9:25       ` Jeff King
  2017-07-11 15:55       ` Junio C Hamano
  2017-07-11 15:50     ` Junio C Hamano
  1 sibling, 2 replies; 17+ messages in thread
From: Lars Schneider @ 2017-07-11  9:18 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git


> On 11 Jul 2017, at 11:11, Jeff King <peff@peff.net> wrote:
> 
> On Tue, Jul 11, 2017 at 10:28:08AM +0200, Lars Schneider wrote:
> 
>>> On 11 Jul 2017, at 00:48, Junio C Hamano <gitster@pobox.com> wrote:
>>> 
>>> * ls/filter-process-delayed (2017-06-30) 7 commits
>>> (merged to 'next' on 2017-07-05 at a35e644082)
>>> + convert: add "status=delayed" to filter process protocol
>>> + convert: refactor capabilities negotiation
>>> + convert: move multiple file filter error handling to separate function
>>> + convert: put the flags field before the flag itself for consistent style
>>> + t0021: write "OUT <size>" only on success
>>> + t0021: make debug log file name configurable
>>> + t0021: keep filter log files on comparison
>>> 
>>> The filter-process interface learned to allow a process with long
>>> latency give a "delayed" response.
>>> 
>>> Will merge to 'master'.
>> 
>> Hi Junio,
>> 
>> can you already tell if this topic has still a chance to be part of 2.14?
> 
> I'm not Junio, but we should be able to reason it out. :)
> 
> It's marked as "will merge to master", which typically means it will
> happen during the next integration cycle (i.e., within a few days when
> the next "What's cooking" is written).
> 
> Since 2.14 will be cut from the tip of master in a few weeks, once it's
> merged it will definitely be in 2.14 (barring a revert, of course).
> 
> This holds true during release freeze, too. Anything that makes it to
> master is part of the release. The stopping point there is that things
> stop getting marked as "will merge to master".

Thanks for the explanation! I looked at the Git release calendar [1] and
realized that 2.14-rc0 is scheduled for this Thursday. My assumption was
that either on this date 2.14 will be cut from the tip of master or master
would not change significantly after the rc0 date until the 2.14 release.

- Lars


[1] http://tinyurl.com/gitCal


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-11  9:18     ` Lars Schneider
@ 2017-07-11  9:25       ` Jeff King
  2017-07-11 15:55       ` Junio C Hamano
  1 sibling, 0 replies; 17+ messages in thread
From: Jeff King @ 2017-07-11  9:25 UTC (permalink / raw)
  To: Lars Schneider; +Cc: Junio C Hamano, git

On Tue, Jul 11, 2017 at 11:18:17AM +0200, Lars Schneider wrote:

> Thanks for the explanation! I looked at the Git release calendar [1] and
> realized that 2.14-rc0 is scheduled for this Thursday. My assumption was
> that either on this date 2.14 will be cut from the tip of master or master
> would not change significantly after the rc0 date until the 2.14 release.

Yeah, certainly forking off a 2.14-rc branch would be a reasonable way
to do the release management. It just happens to not be the way we do
it.

One nice thing about keeping "master" as the stabilizing
release-candidate branch is that it encourages more people to run it
(versus having people manually switch to building a 2.14-rc branch).

-Peff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-11  9:11   ` Jeff King
  2017-07-11  9:18     ` Lars Schneider
@ 2017-07-11 15:50     ` Junio C Hamano
  1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2017-07-11 15:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Lars Schneider, git

Jeff King <peff@peff.net> writes:

> On Tue, Jul 11, 2017 at 10:28:08AM +0200, Lars Schneider wrote:
>
>> > On 11 Jul 2017, at 00:48, Junio C Hamano <gitster@pobox.com> wrote:
>> > 
>> > * ls/filter-process-delayed (2017-06-30) 7 commits
>> >  (merged to 'next' on 2017-07-05 at a35e644082)
>> > + convert: add "status=delayed" to filter process protocol
>> > + convert: refactor capabilities negotiation
>> > + convert: move multiple file filter error handling to separate function
>> > + convert: put the flags field before the flag itself for consistent style
>> > + t0021: write "OUT <size>" only on success
>> > + t0021: make debug log file name configurable
>> > + t0021: keep filter log files on comparison
>> > 
>> > The filter-process interface learned to allow a process with long
>> > latency give a "delayed" response.
>> > 
>> > Will merge to 'master'.
>> 
>> Hi Junio,
>> 
>> can you already tell if this topic has still a chance to be part of 2.14?
>
> I'm not Junio, but we should be able to reason it out. :)

I am ;-).

> It's marked as "will merge to master", which typically means it will
> happen during the next integration cycle (i.e., within a few days when
> the next "What's cooking" is written).

Just like being in 'next' is not a promise for a topic to be in the
upcoming release (or any future one for that matter), "Will merge to
X" merely means "With the current shape of the series and with the
best of our current knowledge, this is thought to be mergeable to X
at some point in the future".  When a more urgent topic that
conflicts heavily with it appears, when a serious bug is found in
the topic, etc., "our current best knowledge" may be invalidated.

Anticipating such an event that changes our assumption happening, I
try to keep a topic in 'next' for at least a week, if it is a
non-trivial topic that changes more than a few dozen lines (not
counting tests and docs).  For things that touch deeper guts of the
system, I'd prefer to cook it longer.  For more trivial things, it
may only be a day or two.  So "at some point in the future" varies
depending on the weight of a topic.

> Since 2.14 will be cut from the tip of master in a few weeks, once
> it's merged it will definitely be in 2.14 (barring a revert, of
> course).  This holds true during release freeze, too. Anything
> that makes it to master is part of the release.

This is correct.

> The stopping point there is that things stop getting marked as
> "will merge to master".

This is correct, if you allow the possibility that a thing that used
to be marked as "Will merge to 'master'" gets demoted to "Will cook
in 'next'" (and you need to anticipate that possibility---I try to
demote topics that got in to 'next' just before -rc0 to to "Will
cook" when tagging -rc0, unless they are fixes that are not too
involved).

I probably should have marked this as "Will cook in 'next'" in the
first place.  The practice has been to blindly promote "Will merge
to 'next'" to "Will merge to 'master'" by default when a topic gets
merged to 'next', and then inside of the first week try to find a
chance to re-read the topic to decide to demote it to "Will cook".

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-11  9:18     ` Lars Schneider
  2017-07-11  9:25       ` Jeff King
@ 2017-07-11 15:55       ` Junio C Hamano
  1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2017-07-11 15:55 UTC (permalink / raw)
  To: Lars Schneider; +Cc: Jeff King, git

Lars Schneider <larsxschneider@gmail.com> writes:

> Thanks for the explanation! I looked at the Git release calendar [1] and
> realized that 2.14-rc0 is scheduled for this Thursday. My assumption was
> that either on this date 2.14 will be cut from the tip of master or master
> would not change significantly after the rc0 date until the 2.14 release.

Your assumption is still correct.  Undercooked topics may be reclassified
to "Will cook in 'next'" any time.

Thanks.

> [1] http://tinyurl.com/gitCal

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-10 22:48 What's cooking in git.git (Jul 2017, #03; Mon, 10) Junio C Hamano
  2017-07-11  8:28 ` Lars Schneider
@ 2017-07-13 12:44 ` Johannes Schindelin
  2017-07-13 15:22   ` Junio C Hamano
  1 sibling, 1 reply; 17+ messages in thread
From: Johannes Schindelin @ 2017-07-13 12:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi Junio,

On Mon, 10 Jul 2017, Junio C Hamano wrote:

> * jc/allow-lazy-cas (2017-07-06) 1 commit
>  - push: disable lazy --force-with-lease by default
> 
>  Because "git push --force-with-lease[=<ref>]" that relies on the
>  stability of remote-tracking branches is unsafe when something
>  fetches into the repository behind user's back, it is now disabled
>  by default.  A new configuration variable can be used to enable it
>  by users who know what they are doing.  This would pave the way to
>  possibly turn `--force` into `--force-with-lease`.
> 
>  Will wait for feedback, then merge to and cook in 'next'.

I wonder whether the --force-with-lease option would benefit (and counter
the "unsafe because of behind-the-back operations" argument) from doing
some kind of "reverse pull --rebase".

In other words, --force-with-lease could verify that the upstream branch
has no commits that weren't seen in the current branch's reflog, i.e. that
`git rev-parse HEAD@{upstream}` is identical to `git merge-base
--fork-point HEAD HEAD@{upstream}`.

The assumption would be that you typically want to push with a lease only
when following the `pull --rebase` workflow. Meaning that you would only
want to force-push when your local branch had the upstream branch
integrated at some stage [*1*].

I could imagine, though, that this should be an opt-in option for now, and
could be turned into an opt-out option later. Or maybe just make it
opt-out, adding a kick-ass error message explaining the situation properly
(and how to override the safe-guard from the command-line).

Ciao,
Dscho

Footnote *1*: Of course, if you merge upstream, do some stuff and then
reset --hard to the previous state, this safeguard will not catch. It
would *still* catch when aborting a rebase onto upstream, though.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-13 12:44 ` Johannes Schindelin
@ 2017-07-13 15:22   ` Junio C Hamano
  2017-07-13 15:53     ` Jeff King
  0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2017-07-13 15:22 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> I wonder whether the --force-with-lease option would benefit (and counter
> the "unsafe because of behind-the-back operations" argument) from doing
> some kind of "reverse pull --rebase".
>
> In other words, --force-with-lease could verify that the upstream branch
> has no commits that weren't seen in the current branch's reflog, i.e. that
> `git rev-parse HEAD@{upstream}` is identical to `git merge-base
> --fork-point HEAD HEAD@{upstream}`.
>
> The assumption would be that you typically want to push with a lease only
> when following the `pull --rebase` workflow. Meaning that you would only
> want to force-push when your local branch had the upstream branch
> integrated at some stage [*1*].

I suspect that the problem "--force-with-lease" wants to solve does
not even appear in "pull --rebase" workflow, but let me think aloud
to see where this leads, as the idea sounds interesting, even though
I may have misunderstood what you meant by the above.

If you do a "pull --rebase", you first rewind to the upstream and
then reapply your changes on top, meaning that the result would
normally be based on what was once at the tip of the upstream and
should fast-forward.  The only situation that a result of "pull
--rebase" needs a non-ff push is when somebody else pushed there in
the meantime, adding commits that you haven't seen yet.  And you
do not want to force your push to lose their work with "--force",
with or without any lease.

It does not change if you do an extra "fetch" to update the tip of
your remote-tracking branch with their work.  Using the lazy
"--force-with-lease" is a wrong thing to do here, of course, but
using any "--force", with or without lease, is _unnecessary_ with
"pull --rebase".  The safety afforded by the bog standard push that
requires fast-forward is sufficient.

And the above will equally apply to a non rebasing "pull".

The problem "with-lease" part tries to solve is when you are
actively rewinding the history of the upstream.  Your upstream may
look like:

    o---A---B---C

and you fetch, squash B and C into one with some changes, to come up
with a local history:

          BC
         /
    o---A---B---C

and try to replace the upstream's tip (which is _supposed to be_ at
C) with BC; the resulting history leading to BC is supposed to be a
replacement of the upstream's history, and that is true only when
the upstream's tip is still at C when you do your push.  If somebody
else built D on top of C and pushed there, that would no longer be
true and you'd end up losing it by replacing the upstream's tip with
your BC.  Before the "with lease" option, when you want to fix the
"---B---C" segment of the history that is already published, the
only way to replace it with a single commit BC was with "--force"
and there is no mechanism other than inter-developer coordination to
make sure nobody will push D on top of C that will be lost with your
forced push.  "push --force-with-lease=<ref>:C BC:<ref>" is meant to
be a solution for that issue.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-13 15:22   ` Junio C Hamano
@ 2017-07-13 15:53     ` Jeff King
  2017-07-13 16:32       ` Junio C Hamano
  2017-07-13 17:51       ` Junio C Hamano
  0 siblings, 2 replies; 17+ messages in thread
From: Jeff King @ 2017-07-13 15:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git

On Thu, Jul 13, 2017 at 08:22:54AM -0700, Junio C Hamano wrote:

> > In other words, --force-with-lease could verify that the upstream branch
> > has no commits that weren't seen in the current branch's reflog, i.e. that
> > `git rev-parse HEAD@{upstream}` is identical to `git merge-base
> > --fork-point HEAD HEAD@{upstream}`.
> >
> > The assumption would be that you typically want to push with a lease only
> > when following the `pull --rebase` workflow. Meaning that you would only
> > want to force-push when your local branch had the upstream branch
> > integrated at some stage [*1*].
> 
> I suspect that the problem "--force-with-lease" wants to solve does
> not even appear in "pull --rebase" workflow, but let me think aloud
> to see where this leads, as the idea sounds interesting, even though
> I may have misunderstood what you meant by the above.
> 
> If you do a "pull --rebase", you first rewind to the upstream and
> then reapply your changes on top, meaning that the result would
> normally be based on what was once at the tip of the upstream and
> should fast-forward.  The only situation that a result of "pull
> --rebase" needs a non-ff push is when somebody else pushed there in
> the meantime, adding commits that you haven't seen yet.  And you
> do not want to force your push to lose their work with "--force",
> with or without any lease.

That assumes that you're pushing back to the upstream. You could be
doing a "pull --rebase" from upstream and then pushing up to another ref
(or even another repo).

Consider something like a topic branch whose @{upstream} is
origin/master, but you use the "current" push.default. If you do:

  $ git pull --rebase
  $ git push

then the push is a non-ff, because you're pushing the rebased commits.
You'd ideally want a way to force that without clobbering a push that
somebody else made to the topic branch.

I do think Dscho is on to something with the "see if it was ever in our
reflog" idea. What you really want as a default for force-with-lease is
some way to say "I want to change that ref from X to Y, but only for
values of X that I know Y has already considered and incorporated". So
the danger in using the tracking branch is that you may not have
incorporated that value (either because somebody fetched behind your
back, or because you simply forgot to merge it into your local branch
when doing your history-rewriting operation).

So even forgetting "git pull --rebase", doing:

  $ git fetch
  $ git rebase HEAD~2
  $ git push --force-with-lease

is bad, because your rebase didn't take into account what was fetched in
step 1. But imagine force-with-lease rule were more like: when pushing
branch "foo" to remote branch "bar", allow only if the current value of
"bar" is an ancestor of some entry in the reflog of "foo".

That degrades to the usual fast-forward rule if you just check the ref
tip.  And when checking the reflog entries, it shows that at some point
you had your local branch set to something that covered all of the
destination, but then later rewound or rewrote history. We don't have to
care what that operation was. "rebase" is a likely one, but "git reset
--hard HEAD^ && git push" would fall out naturally from the same rule.

It's not quite as safe as a true force-with-lease with a value would be.
For instance, if _upstream_ rewound by a commit, we'd happily undo that
rewind. But then, so does a normal non-forced fast-forward push.

So I'd say this is not really force-with-lease at all, but rather a
special mode that is somewhere between a normal fast-forward and a true
force-with-lease. I don't know if it would make sense to give it a
different name, or just lump it under force-with-lease.

-Peff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-13 15:53     ` Jeff King
@ 2017-07-13 16:32       ` Junio C Hamano
  2017-07-13 17:51       ` Junio C Hamano
  1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2017-07-13 16:32 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, Git Mailing List

On Thu, Jul 13, 2017 at 8:53 AM, Jeff King <peff@peff.net> wrote:
> So I'd say this is not really force-with-lease at all, but rather a
> special mode that is somewhere between a normal fast-forward and a true
> force-with-lease. I don't know if it would make sense to give it a
> different name, or just lump it under force-with-lease.

Yes. I think these are separate problems that needs separate mechanisms
to solve, and I agree with you that it is also a separate issue if a single UI
"safer non-ff push" that covers both (and other problems that will be
discovered later) problems is desired or if a separate options (e.g.
"--force=lease=...,reflog=...," with nicer names and syntax) is better.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-13 15:53     ` Jeff King
  2017-07-13 16:32       ` Junio C Hamano
@ 2017-07-13 17:51       ` Junio C Hamano
  2017-07-13 18:13         ` Jeff King
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2017-07-13 17:51 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, git

Jeff King <peff@peff.net> writes:

> I do think Dscho is on to something with the "see if it was ever in our
> reflog" idea.

Yes, I found the idea was interesting when I responded with the
"thinking aloud", and I still do think it has some uses elsewhere,
even if not in "pull --rebase" workflow.

> What you really want as a default for force-with-lease is
> some way to say "I want to change that ref from X to Y, but only for
> values of X that I know Y has already considered and incorporated".

Correct.

> But imagine force-with-lease rule were more like: when pushing
> branch "foo" to remote branch "bar", allow only if the current value of
> "bar" is an ancestor of some entry in the reflog of "foo".

Would that cover the case that originally motivated the "with lease"
thing?  Let's see.

          BC            "foo"
         /
    o---A---B---C       "bar"

You are pushing back to "bar" that still has C (or it may have
acquired D) from "foo" that is now at BC.  It is likely that you
started to prepare the "A---BC" history from "A---B---C" history, in
which case your current branch "foo" that is being pushed out would
have had C at its tip some time in the past.  So seeing that the tip
of the remote is still C, which is in the reflog of "foo", we can
happily push it out.  If the tip of the remote "bar" is now D
because somebody updated it, you did not consider it while preparing
your BC, and we want to fail the push---because D does not appear in
the reflog of "foo", that is achieved.

> That degrades to the usual fast-forward rule if you just check the ref
> tip.  And when checking the reflog entries, it shows that at some point
> you had your local branch set to something that covered all of the
> destination, but then later rewound or rewrote history. We don't have to
> care what that operation was. "rebase" is a likely one, but "git reset
> --hard HEAD^ && git push" would fall out naturally from the same rule.
>
> It's not quite as safe as a true force-with-lease with a value would be.

All correct.

I wonder if this could be a replacement for the current "the user
did not even specify what the expected current value is, so we
pretend as if the tip of the remote-tracking branch was given"
kludge.  Instead, 

	git push --force-with-lease origin master
	git push --force-with-lease origin topic:master
	git push --force-with-lease origin HEAD:master

could

 (1) first learn where 'refs/heads/master' over there is at.  Call
     it X (it may be C or D in the earlier example).

 (2) locate from which ref the commit we are pushing out is taken;
     in the above examples, they are our refs/heads/master,
     refs/heads/topic, and HEAD, respectively.  Call it R.

 (3) see if the reflog of R has X.  If so do a --force push;
     otherwise fail.

With such an update, I suspect that it would become a lot safer than
relying on the stability of the remote tracking branch, which is
what the current code does.

As you said, this is not as safe as a true "the user knows and tells
what commit was considered" force-with-lease, but can be a lot safer
and can become a replacement for the current "the user does not tell
and allows us to DWIM".

A good thing is that the DWIM is advertised like so:

    ... are still experimental and their semantics may change

so we are free to change it to any better version.  And I think
Dscho's idea could be that better one.

I am still somewhat reserved because in the above, I only
illustrated a case that would work better than the current one,
without trying hard to poke a hole at the reasoning and to come up
with cases that would work worse.  But I agree that this is on to
something good ;-)





    



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-13 17:51       ` Junio C Hamano
@ 2017-07-13 18:13         ` Jeff King
  2017-07-13 19:10           ` Junio C Hamano
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff King @ 2017-07-13 18:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git

On Thu, Jul 13, 2017 at 10:51:21AM -0700, Junio C Hamano wrote:

> > But imagine force-with-lease rule were more like: when pushing
> > branch "foo" to remote branch "bar", allow only if the current value of
> > "bar" is an ancestor of some entry in the reflog of "foo".
> 
> Would that cover the case that originally motivated the "with lease"
> thing?  Let's see.
> 
>           BC            "foo"
>          /
>     o---A---B---C       "bar"
> 
> You are pushing back to "bar" that still has C (or it may have
> acquired D) from "foo" that is now at BC.  It is likely that you
> started to prepare the "A---BC" history from "A---B---C" history, in
> which case your current branch "foo" that is being pushed out would
> have had C at its tip some time in the past.  So seeing that the tip
> of the remote is still C, which is in the reflog of "foo", we can
> happily push it out.  If the tip of the remote "bar" is now D
> because somebody updated it, you did not consider it while preparing
> your BC, and we want to fail the push---because D does not appear in
> the reflog of "foo", that is achieved.

Yeah. I think where this scheme falls down is if you incorporate work
from the remote ref _without_ it ever being in your ref[1]. So for
example I could do something like:

    $ git fetch  ;# new commits arrive
    $ git log origin/master ;# oh my, that tip commit looks broken
    $ git reset --hard origin/master^  ;# lets get rid of it
    $ git push --force-with-lease

and that tricks the system. But the nice thing is that it tricks it in
the "safe" direction. We'd refuse such a push, unless you do it more
like:

    $ git merge origin/master
    $ git reset --hard HEAD^
    $ git push --force-with-lease

Which is really the sane way to do it in the first place.

What I'd wonder more is if there are cases where we fail in the unsafe
direction. Here's the most plausible scenario I could come up with:

  $ git pull ;# get some new commits
  $ git reset --hard HEAD^ ;# doh, that merge didn't look right
  $ git rebase -i ;# hack hack hack
  $ git push --force-with-lease ;# oops, I should have re-merged

We get fooled because yes, we were once on the remote's tip. But we
discarded it and then did a _different_ rewriting operation (of course
it is impossible for the tool to tell if the original "reset --hard" was
an intentional rewriting operation, or one where we simply backed out a
mistake).

[1] Another sticking point is that this really does need to be in the
    reflog of the ref we are pushing (and not, e.g., HEAD). But one does
    not always push from a ref. I suspect that's OK in practice, though.
    If you are doing "git push --force-with-lease HEAD~2:master", it is
    probably OK for us to error out with "uh, lease from what?".

> I wonder if this could be a replacement for the current "the user
> did not even specify what the expected current value is, so we
> pretend as if the tip of the remote-tracking branch was given"
> kludge.

Yes, that is exactly what I was thinking of (and why I said that even
though this really isn't force-with-lease in a strict sense, it slots
into the same level of safety, so it might be worth using the name).

> Instead,
> 
> 	git push --force-with-lease origin master
> 	git push --force-with-lease origin topic:master
> 	git push --force-with-lease origin HEAD:master
> 
> could
> 
>  (1) first learn where 'refs/heads/master' over there is at.  Call
>      it X (it may be C or D in the earlier example).
> 
>  (2) locate from which ref the commit we are pushing out is taken;
>      in the above examples, they are our refs/heads/master,
>      refs/heads/topic, and HEAD, respectively.  Call it R.
> 
>  (3) see if the reflog of R has X.  If so do a --force push;
>      otherwise fail.

Yes, more or less. A few thoughts:

  - that step 2 is where the "wait, that isn't even a ref" error above
    would come in

  - I suspect in the third example we probably ought to be using the
    reflog of the branch that HEAD points to. You would not want:

       $ git checkout advanced-branch $ git checkout older-branch $ git
       push --force-with-lease origin HEAD:older-branch

    to consider that commits in advanced-branch are part of the lease.

  - For step 3, I'm not sure if we you mean to look for exactly X, or
    if it would be OK to have any commit whose ancestor is X. I think
    you'd need the latter to accommodate a non-fast-forward "git pull"
    (or fetch+merge) where the local ref is never set precisely to the
    upstream commit.

-Peff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-13 18:13         ` Jeff King
@ 2017-07-13 19:10           ` Junio C Hamano
  2017-07-13 19:39             ` Junio C Hamano
  2017-07-13 20:49             ` Jeff King
  0 siblings, 2 replies; 17+ messages in thread
From: Junio C Hamano @ 2017-07-13 19:10 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, git

Jeff King <peff@peff.net> writes:

> [1] Another sticking point is that this really does need to be in the
>     reflog of the ref we are pushing (and not, e.g., HEAD). But one does
>     not always push from a ref. I suspect that's OK in practice, though.
>     If you are doing "git push --force-with-lease HEAD~2:master", it is
>     probably OK for us to error out with "uh, lease from what?".

I actually expect this to bite me personally, as I often do not
rewind the actual "topic" ref that is my target of rewriting,
because I am a chicken---I detach the HEAD and build a history there
to see if I can come up with a better history, compare the result
with what is on the "topic" (which is not touched at all during the
rewriting), and after all is done, do a "checkout -B topic".  The
"remote tip must appear in the local reflog" rule will never be
satisfied.

>> I wonder if this could be a replacement for the current "the user
>> did not even specify what the expected current value is, so we
>> pretend as if the tip of the remote-tracking branch was given"
>> kludge.
>
> Yes, that is exactly what I was thinking of (and why I said that even
> though this really isn't force-with-lease in a strict sense, it slots
> into the same level of safety, so it might be worth using the name).
>
>> Instead,
>> 
>> 	git push --force-with-lease origin master
>> 	git push --force-with-lease origin topic:master
>> 	git push --force-with-lease origin HEAD:master
>> 
>> could
>> 
>>  (1) first learn where 'refs/heads/master' over there is at.  Call
>>      it X (it may be C or D in the earlier example).
>> 
>>  (2) locate from which ref the commit we are pushing out is taken;
>>      in the above examples, they are our refs/heads/master,
>>      refs/heads/topic, and HEAD, respectively.  Call it R.
>> 
>>  (3) see if the reflog of R has X.  If so do a --force push;
>>      otherwise fail.
>
> Yes, more or less. A few thoughts:
>
>   - that step 2 is where the "wait, that isn't even a ref" error above
>     would come in
>
>   - I suspect in the third example we probably ought to be using the
>     reflog of the branch that HEAD points to. You would not want:
>
>        $ git checkout advanced-branch $ git checkout older-branch $ git
>        push --force-with-lease origin HEAD:older-branch
>
>     to consider that commits in advanced-branch are part of the lease.

The third one was meant to be rewriting on detached HEAD, not having
any underlying branch.

>   - For step 3, I'm not sure if we you mean to look for exactly X, or
>     if it would be OK to have any commit whose ancestor is X. I think
>     you'd need the latter to accommodate a non-fast-forward "git pull"
>     (or fetch+merge) where the local ref is never set precisely to the
>     upstream commit.

But the result in that case is a descendant of upstream you just
merged, so you do not even want to use any form of forcing---you
would rather want to rely on the usual "push must fast-forward"
mechanism, no?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-13 19:10           ` Junio C Hamano
@ 2017-07-13 19:39             ` Junio C Hamano
  2017-07-13 20:49             ` Jeff King
  1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2017-07-13 19:39 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, git

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> [1] Another sticking point is that this really does need to be in the
>>     reflog of the ref we are pushing (and not, e.g., HEAD). But one does
>>     not always push from a ref. I suspect that's OK in practice, though.
>>     If you are doing "git push --force-with-lease HEAD~2:master", it is
>>     probably OK for us to error out with "uh, lease from what?".
>
> I actually expect this to bite me personally, as I often do not
> rewind the actual "topic" ref that is my target of rewriting,
> because I am a chicken---I detach the HEAD and build a history there
> to see if I can come up with a better history, compare the result
> with what is on the "topic" (which is not touched at all during the
> rewriting), and after all is done, do a "checkout -B topic".  The
> "remote tip must appear in the local reflog" rule will never be
> satisfied.

Well "bite" is not quite accurate, as I do not push to a repository
racing with others, and there is no reason for me to use --force
with lease.  I always push '+pu', and push '+next" once after each
release, and there is no need for any lease when I push things out.

But if I were collaborating with others in another project that uses
a shared central repository workflow, and if I were in the mood to
see how well/better this "improved safer DWIM" mode behaves, then it
would bite me.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-13 19:10           ` Junio C Hamano
  2017-07-13 19:39             ` Junio C Hamano
@ 2017-07-13 20:49             ` Jeff King
  2017-07-13 21:24               ` Junio C Hamano
  1 sibling, 1 reply; 17+ messages in thread
From: Jeff King @ 2017-07-13 20:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git

On Thu, Jul 13, 2017 at 12:10:39PM -0700, Junio C Hamano wrote:

> >   - I suspect in the third example we probably ought to be using the
> >     reflog of the branch that HEAD points to. You would not want:
> >
> >        $ git checkout advanced-branch $ git checkout older-branch $ git
> >        push --force-with-lease origin HEAD:older-branch
> >
> >     to consider that commits in advanced-branch are part of the lease.
> 
> The third one was meant to be rewriting on detached HEAD, not having
> any underlying branch.

Ah, OK, that makes a lot more sense. I'd be tempted to say that it
should not allow force-with-lease at all. The HEAD reflog sees too many
unrelated things to make it a good basis for "does this result account
for everything in its reflog".

> >   - For step 3, I'm not sure if we you mean to look for exactly X, or
> >     if it would be OK to have any commit whose ancestor is X. I think
> >     you'd need the latter to accommodate a non-fast-forward "git pull"
> >     (or fetch+merge) where the local ref is never set precisely to the
> >     upstream commit.
> 
> But the result in that case is a descendant of upstream you just
> merged, so you do not even want to use any form of forcing---you
> would rather want to rely on the usual "push must fast-forward"
> mechanism, no?

Sorry, I mean the case where you do a merge from the other side, but
then you end up rewinding the history in some way, taking into account
that merge and everything they did. For example:

  $ git pull
  $ git rebase  ;# this will flatten the merge
  $ git push --force-with-lease

There was never a moment where the other side's tip ref was in your
local branch, but you did incorporate it via the merge.

-Peff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)
  2017-07-13 20:49             ` Jeff King
@ 2017-07-13 21:24               ` Junio C Hamano
  0 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2017-07-13 21:24 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, git

Jeff King <peff@peff.net> writes:

> Sorry, I mean the case where you do a merge from the other side, but
> then you end up rewinding the history in some way, taking into account
> that merge and everything they did. For example:
>
>   $ git pull
>   $ git rebase  ;# this will flatten the merge
>   $ git push --force-with-lease
>
> There was never a moment where the other side's tip ref was in your
> local branch, but you did incorporate it via the merge.

Ah, OK, now it is clear what you meant.

Thanks.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2017-07-13 21:24 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-10 22:48 What's cooking in git.git (Jul 2017, #03; Mon, 10) Junio C Hamano
2017-07-11  8:28 ` Lars Schneider
2017-07-11  9:11   ` Jeff King
2017-07-11  9:18     ` Lars Schneider
2017-07-11  9:25       ` Jeff King
2017-07-11 15:55       ` Junio C Hamano
2017-07-11 15:50     ` Junio C Hamano
2017-07-13 12:44 ` Johannes Schindelin
2017-07-13 15:22   ` Junio C Hamano
2017-07-13 15:53     ` Jeff King
2017-07-13 16:32       ` Junio C Hamano
2017-07-13 17:51       ` Junio C Hamano
2017-07-13 18:13         ` Jeff King
2017-07-13 19:10           ` Junio C Hamano
2017-07-13 19:39             ` Junio C Hamano
2017-07-13 20:49             ` Jeff King
2017-07-13 21:24               ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.