* What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Junio C Hamano @ 2017-01-16 1:51 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.
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]
* bw/read-blob-data-does-not-modify-index-state (2017-01-11) 1 commit
- index: improve constness for reading blob data
Code clean-up.
Will merge to 'next'.
* ep/commit-static-buf-cleanup (2017-01-13) 2 commits
- builtin/commit.c: switch to xstrfmt(), instead of snprintf()
- builtin/commit.c: remove the PATH_MAX limitation via dynamic allocation
Code clean-up.
Expecting a reroll.
The tip one would instead be done with strbuf.
cf. <CA+EOSB=4-TKpi6mr-yVbwRsFrVzE=vo4Y9Qm3VMm7pn=UB1_hQ@mail.gmail.com>
* jk/grep-e-could-be-extended-beyond-posix (2017-01-11) 1 commit
- t7810: avoid assumption about invalid regex syntax
Tighten a test to avoid mistaking an extended ERE regexp engine as
a PRE regexp engine.
Will merge to 'next'.
* jk/vreport-sanitize (2017-01-11) 2 commits
- vreport: sanitize ASCII control chars
- Revert "vreportf: avoid intermediate buffer"
An error message with an ASCII control character like '\r' in it
can alter the message to hide its early part, which is problematic
when a remote side gives such an error message that the local side
will relay with a "remote: " prefix.
Will merge to 'next'.
* sb/unpack-trees-super-prefix (2017-01-12) 5 commits
- SQUASH
- unpack-trees: support super-prefix option
- t1001: modernize style
- t1000: modernize style
- read-tree: use OPT_BOOL instead of OPT_SET_INT
"git read-tree" and its underlying unpack_trees() machinery learned
to report problematic paths prefixed with the --super-prefix option.
Expecting a reroll.
The first three are in good shape. The last one needs a better
explanation and possibly an update to its test.
cf. <CAGZ79kaHDnVLQr8WLoaD5KKs7EqeW=KbkptF=iHpU5t054Xcdw@mail.gmail.com>
* bw/grep-recurse-submodules (2016-12-22) 12 commits
(merged to 'next' on 2016-12-22 at 1ede815b8d)
+ grep: search history of moved submodules
+ grep: enable recurse-submodules to work on <tree> objects
+ grep: optionally recurse into submodules
+ grep: add submodules as a grep source type
+ submodules: load gitmodules file from commit sha1
+ submodules: add helper to determine if a submodule is initialized
+ submodules: add helper to determine if a submodule is populated
(merged to 'next' on 2016-12-22 at fea8fa870f)
+ real_path: canonicalize directory separators in root parts
+ real_path: have callers use real_pathdup and strbuf_realpath
+ real_path: create real_pathdup
+ real_path: convert real_path_internal to strbuf_realpath
+ real_path: resolve symlinks by hand
(this branch is tangled with bw/realpath-wo-chdir.)
"git grep" has been taught to optionally recurse into submodules.
Will merge to 'master'.
* sb/submodule-config-tests (2017-01-12) 2 commits
- t7411: test lookup of uninitialized submodules
- t7411: quote URLs
Will merge to 'next'.
* sb/submodule-doc (2017-01-12) 3 commits
- submodules: add a background story
- submodule update documentation: don't repeat ourselves
- submodule documentation: add options to the subcommand
Needs review.
* sb/submodule-embed-gitdir (2017-01-12) 1 commit
- submodule absorbgitdirs: mention in docstring help
Help-text fix.
Will merge to 'next'.
* sb/submodule-init (2017-01-12) 1 commit
- submodule update --init: display correct path from submodule
Error message fix.
Will merge to 'next'.
* vn/diff-ihc-config (2017-01-12) 1 commit
- diff: add interhunk context config option
"git diff" learned diff.interHunkContext configuration variable
that gives the default value for its --inter-hunk-context option.
Will merge to 'next'.
* ad/bisect-terms (2017-01-13) 1 commit
- Documentation/bisect: improve on (bad|new) and (good|bad)
Documentation fix.
Will merge to 'next'.
* bw/attr (2017-01-15) 27 commits
- attr: reformat git_attr_set_direction() function
- attr: push the bare repo check into read_attr()
- attr: store attribute stacks in hashmap
- attr: tighten const correctness with git_attr and match_attr
- attr: remove maybe-real, maybe-macro from git_attr
- attr: eliminate global check_all_attr array
- attr: use hashmap for attribute dictionary
- attr: change validity check for attribute names to use positive logic
- attr: pass struct attr_check to collect_some_attrs
- attr: retire git_check_attrs() API
- attr: convert git_check_attrs() callers to use the new API
- attr: convert git_all_attrs() to use "struct attr_check"
- attr: (re)introduce git_check_attr() and struct attr_check
- attr: rename function and struct related to checking attributes
- attr.c: outline the future plans by heavily commenting
- Documentation/gitattributes.txt: fix a typo
- attr.c: add push_stack() helper
- attr: support quoting pathname patterns in C style
- attr.c: plug small leak in parse_attr_line()
- attr.c: tighten constness around "git_attr" structure
- attr.c: simplify macroexpand_one()
- attr.c: mark where #if DEBUG ends more clearly
- attr.c: complete a sentence in a comment
- attr.c: explain the lack of attr-name syntax check in parse_attr()
- attr.c: update a stale comment on "struct match_attr"
- attr.c: use strchrnul() to scan for one line
- commit.c: use strchrnul() to scan for one line
The gitattributes machinery is being taught to work better in a
multi-threaded environment.
Needs review.
* jk/loose-object-fsck (2017-01-15) 6 commits
- fsck: detect trailing garbage in all object types
- fsck: parse loose object paths directly
- sha1_file: add read_loose_object() function
- t1450: test fsck of packed objects
- sha1_file: fix error message for alternate objects
- t1450: refactor loose-object removal
"git fsck" inspects loose objects more carefully now.
Needs review.
* rh/diff-orderfile-doc (2017-01-15) 2 commits
- diff: document the format of the -O (diff.orderFile) file
- diff: document behavior of relative diff.orderFile
Documentation fix.
Will merge to 'next'.
* sb/cd-then-git-can-be-written-as-git-c (2017-01-13) 1 commit
- lib-submodule-update.sh: reduce use of subshell by using "git -C"
Test clean-up.
Will merge to 'next'.
* vn/xdiff-func-context (2017-01-15) 1 commit
- xdiff -W: relax end-of-file function detection
"git diff -W" has been taught to handle the case where a new
function is added at the end of the file better.
Will hold.
An follow-up change to go back from the line that matches the
funcline to show comments before the function definition is still
being discussed.
* ws/request-pull-code-cleanup (2017-01-15) 1 commit
- request-pull: drop old USAGE stuff
Code clean-up.
Will merge to 'next'.
--------------------------------------------------
[Stalled]
* jk/nofollow-attr-ignore (2016-11-02) 5 commits
. exclude: do not respect symlinks for in-tree .gitignore
. attr: do not respect symlinks for in-tree .gitattributes
. exclude: convert "check_index" into a flags field
. attr: convert "macro_ok" into a flags field
. add open_nofollow() helper
As we do not follow symbolic links when reading control files like
.gitignore and .gitattributes from the index, match the behaviour
and not follow symbolic links when reading them from the working
tree. This also tightens security a bit by not leaking contents of
an unrelated file in the error messages when it is pointed at by
one of these files that is a symbolic link.
Perhaps we want to cover .gitmodules too with the same mechanism?
Ejected for now.
* jc/bundle (2016-03-03) 6 commits
- index-pack: --clone-bundle option
- Merge branch 'jc/index-pack' into jc/bundle
- bundle v3: the beginning
- bundle: keep a copy of bundle file name in the in-core bundle header
- bundle: plug resource leak
- bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
While I think it would make it easier for people to experiment and
build on if the topic is merged to 'next', I am at the same time a
bit reluctant to merge an unproven new topic that introduces a new
file format, which we may end up having to support til the end of
time. It is likely that to support a "prime clone from CDN", it
would need a lot more than just "these are the heads and the pack
data is over there", so this may not be sufficient.
Will discard.
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* ls/p4-retry-thrice (2016-12-29) 1 commit
(merged to 'next' on 2017-01-10 at c733e27410)
+ git-p4: do not pass '-r 0' to p4 commands
A recent updates to "git p4" was not usable for older p4 but it
could be made to work with minimum changes. Do so.
Will merge to 'master'.
* mh/ref-remove-empty-directory (2017-01-07) 23 commits
- files_transaction_commit(): clean up empty directories
- try_remove_empty_parents(): teach to remove parents of reflogs, too
- try_remove_empty_parents(): don't trash argument contents
- try_remove_empty_parents(): rename parameter "name" -> "refname"
- delete_ref_loose(): inline function
- delete_ref_loose(): derive loose reference path from lock
- log_ref_write_1(): inline function
- log_ref_setup(): manage the name of the reflog file internally
- log_ref_write_1(): don't depend on logfile argument
- log_ref_setup(): pass the open file descriptor back to the caller
- log_ref_setup(): improve robustness against races
- log_ref_setup(): separate code for create vs non-create
- log_ref_write(): inline function
- rename_tmp_log(): improve error reporting
- rename_tmp_log(): use raceproof_create_file()
- lock_ref_sha1_basic(): use raceproof_create_file()
- lock_ref_sha1_basic(): inline constant
- raceproof_create_file(): new function
- safe_create_leading_directories(): set errno on SCLD_EXISTS
- safe_create_leading_directories_const(): preserve errno
- t5505: use "for-each-ref" to test for the non-existence of references
- refname_is_safe(): correct docstring
- files_rename_ref(): tidy up whitespace
Deletion of a branch "foo/bar" could remove .git/refs/heads/foo
once there no longer is any other branch whose name begins with
"foo/", but we didn't do so so far. Now we do.
Expecting a reroll.
cf. <5051c78e-51f9-becd-e1a6-9c0b781d6912@alum.mit.edu>
* pb/test-must-fail-is-for-git (2017-01-09) 2 commits
(merged to 'next' on 2017-01-10 at 5f24a98779)
+ t9813: avoid using pipes
+ don't use test_must_fail with grep
Test cleanup.
Will merge to 'master'.
* jk/archive-zip-userdiff-config (2017-01-07) 1 commit
(merged to 'next' on 2017-01-10 at ac42e4958c)
+ archive-zip: load userdiff config
"git archive" did not read the standard configuration files, and
failed to notice a file that is marked as binary via the userdiff
driver configuration.
Will merge to 'master'.
* jk/blame-fixes (2017-01-07) 3 commits
(merged to 'next' on 2017-01-10 at 18f909da61)
+ blame: output porcelain "previous" header for each file
+ blame: handle --no-abbrev
+ blame: fix alignment with --abbrev=40
"git blame --porcelain" misidentified the "previous" <commit, path>
pair (aka "source") when contents came from two or more files.
Will merge to 'master'.
* jk/rebase-i-squash-count-fix (2017-01-07) 1 commit
(merged to 'next' on 2017-01-10 at d6cfc6ace2)
+ rebase--interactive: count squash commits above 10 correctly
"git rebase -i" with a recent update started showing an incorrect
count when squashing more than 10 commits.
Will merge to 'master'.
* js/asciidoctor-tweaks (2017-01-13) 2 commits
(merged to 'next' on 2017-01-15 at 8553c6e513)
+ asciidoctor: fix user-manual to be built by `asciidoctor`
(merged to 'next' on 2017-01-10 at 087da7b7c1)
+ giteveryday: unbreak rendering with AsciiDoctor
Adjust documentation to help AsciiDoctor render better while not
breaking the rendering done by AsciiDoc.
Will merge to 'master'.
* km/branch-get-push-while-detached (2017-01-07) 1 commit
(merged to 'next' on 2017-01-10 at a7f8af8c55)
+ branch_get_push: do not segfault when HEAD is detached
"git <cmd> @{push}" on a detached HEAD used to segfault; it has
been corrected to error out with a message.
Will merge to 'master'.
* sb/remove-gitview (2017-01-13) 4 commits
(merged to 'next' on 2017-01-15 at 7c9eae479e)
+ doc: git-gui browser does not default to HEAD
+ doc: gitk: add the upstream repo location
+ doc: gitk: remove gitview reference
(merged to 'next' on 2017-01-10 at dcb3abd146)
+ contrib: remove gitview
Will merge to 'master'.
* sb/submodule-cleanup-export-git-dir-env (2017-01-07) 1 commit
(merged to 'next' on 2017-01-10 at 2d5db6821e)
+ submodule.c: use GIT_DIR_ENVIRONMENT consistently
Code cleanup.
Will merge to 'master'.
* sb/pathspec-errors (2017-01-09) 1 commit
(merged to 'next' on 2017-01-10 at 432375cb62)
+ pathspec: give better message for submodule related pathspec error
(this branch uses bw/pathspec-cleanup.)
Running "git add a/b" when "a" is a submodule correctly errored
out, but without a meaningful error message.
Will merge to 'master'.
* ls/filter-process-delayed (2017-01-08) 1 commit
. convert: add "status=delayed" to filter process protocol
Ejected, as does not build when merged to 'pu'.
* sp/cygwin-build-fixes (2017-01-09) 2 commits
(merged to 'next' on 2017-01-10 at 2010fb6c03)
+ Makefile: put LIBS after LDFLAGS for imap-send
+ Makefile: POSIX windres
Build updates for Cygwin.
Will merge to 'master'.
* jk/execv-dashed-external (2017-01-09) 3 commits
(merged to 'next' on 2017-01-10 at 117b506cb0)
+ execv_dashed_external: wait for child on signal death
+ execv_dashed_external: stop exiting with negative code
+ execv_dashed_external: use child_process struct
Typing ^C to pager, which usually does not kill it, killed Git and
took the pager down as a collateral damage in certain process-tree
structure. This has been fixed.
Will merge to 'master'.
* rh/mergetool-regression-fix (2017-01-10) 14 commits
(merged to 'next' on 2017-01-10 at e8e00c798b)
+ mergetool: fix running in subdir when rerere enabled
+ mergetool: take the "-O" out of $orderfile
+ t7610: add test case for rerere+mergetool+subdir bug
+ t7610: spell 'git reset --hard' consistently
+ t7610: don't assume the checked-out commit
+ t7610: always work on a test-specific branch
+ t7610: delete some now-unnecessary 'git reset --hard' lines
+ t7610: run 'git reset --hard' after each test to clean up
+ t7610: don't rely on state from previous test
+ t7610: use test_when_finished for cleanup tasks
+ t7610: move setup code to the 'setup' test case
+ t7610: update branch names to match test number
+ rev-parse doc: pass "--" to rev-parse in the --prefix example
+ .mailmap: record canonical email for Richard Hansen
"git mergetool" without any pathspec on the command line that is
run from a subdirectory became no-op in Git v2.11 by mistake, which
has been fixed.
Will merge to 'master'.
* sb/unpack-trees-cleanup (2017-01-10) 3 commits
(merged to 'next' on 2017-01-10 at 95a5f3127c)
+ unpack-trees: factor progress setup out of check_updates
+ unpack-trees: remove unneeded continue
+ unpack-trees: move checkout state into check_updates
Code cleanup.
Will merge to 'master'.
* nd/worktree-move (2017-01-09) 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.
* dt/disable-bitmap-in-auto-gc (2016-12-29) 2 commits
(merged to 'next' on 2017-01-10 at 9f4e89e15d)
+ repack: die on incremental + write-bitmap-index
+ auto gc: don't write bitmaps for incremental repacks
It is natural that "git gc --auto" may not attempt to pack
everything into a single pack, and there is no point in warning
when the user has configured the system to use the pack bitmap,
leading to disabling further "gc".
Will merge to 'master'.
* js/mingw-test-push-unc-path (2017-01-07) 1 commit
(merged to 'next' on 2017-01-10 at 249d9f26f3)
+ mingw: add a regression test for pushing to UNC paths
"git push \\server\share\dir" has recently regressed and then
fixed. A test has retroactively been added for this breakage.
Will merge to 'master'.
* nd/log-graph-configurable-colors (2017-01-08) 1 commit
- log --graph: customize the graph lines with config log.graphColors
Some people feel the default set of colors used by "git log --graph"
rather limiting. A mechanism to customize the set of colors has
been introduced.
Waiting for review comments to be addressed.
cf. <20170109103258.25341-1-pclouds@gmail.com>
* sb/submodule-rm-absorb (2016-12-27) 4 commits
(merged to 'next' on 2017-01-10 at 1fc2000a92)
+ rm: absorb a submodules git dir before deletion
+ submodule: rename and add flags to ok_to_remove_submodule
+ submodule: modernize ok_to_remove_submodule to use argv_array
+ submodule.h: add extern keyword to functions
"git rm" used to refuse to remove a submodule when it has its own
git repository embedded in its working tree. It learned to move
the repository away to $GIT_DIR/modules/ of the superproject
instead, and allow the submodule to be deleted (as long as there
will be no loss of local modifications, that is).
Will merge to 'master'.
* cc/split-index-config (2016-12-26) 21 commits
- Documentation/git-update-index: explain splitIndex.*
- Documentation/config: add splitIndex.sharedIndexExpire
- read-cache: use freshen_shared_index() in read_index_from()
- read-cache: refactor read_index_from()
- t1700: test shared index file expiration
- read-cache: unlink old sharedindex files
- config: add git_config_get_expiry() from gc.c
- read-cache: touch shared index files when used
- sha1_file: make check_and_freshen_file() non static
- Documentation/config: add splitIndex.maxPercentChange
- t1700: add tests for splitIndex.maxPercentChange
- read-cache: regenerate shared index if necessary
- config: add git_config_get_max_percent_split_change()
- Documentation/git-update-index: talk about core.splitIndex config var
- Documentation/config: add information for core.splitIndex
- t1700: add tests for core.splitIndex
- update-index: warn in case of split-index incoherency
- read-cache: add and then use tweak_split_index()
- split-index: add {add,remove}_split_index() functions
- config: add git_config_get_split_index()
- config: mark an error message up for translation
The experimental "split index" feature has gained a few
configuration variables to make it easier to use.
Waiting for review comments to be addressed.
cf. <20161226102222.17150-1-chriscool@tuxfamily.org>
cf. <a1a44640-ff6c-2294-72ac-46322eff8505@ramsayjones.plus.com>
* bw/push-submodule-only (2016-12-20) 3 commits
- push: add option to push only submodules
- submodules: add RECURSE_SUBMODULES_ONLY value
- transport: reformat flag #defines to be more readable
"git submodule push" learned "--recurse-submodules=only option to
push submodules out without pushing the top-level superproject.
* ls/p4-path-encoding (2016-12-18) 1 commit
- git-p4: fix git-p4.pathEncoding for removed files
When "git p4" imports changelist that removes paths, it failed to
convert pathnames when the p4 used encoding different from the one
used on the Git side. This has been corrected.
Will be rerolled.
cf. <7E1C7387-4F37-423F-803D-3B5690B49D40@gmail.com>
* bw/pathspec-cleanup (2017-01-08) 16 commits
(merged to 'next' on 2017-01-10 at 79291ff506)
+ pathspec: rename prefix_pathspec to init_pathspec_item
+ pathspec: small readability changes
+ pathspec: create strip submodule slash helpers
+ pathspec: create parse_element_magic helper
+ pathspec: create parse_long_magic function
+ pathspec: create parse_short_magic function
+ pathspec: factor global magic into its own function
+ pathspec: simpler logic to prefix original pathspec elements
+ pathspec: always show mnemonic and name in unsupported_magic
+ pathspec: remove unused variable from unsupported_magic
+ pathspec: copy and free owned memory
+ pathspec: remove the deprecated get_pathspec function
+ ls-tree: convert show_recursive to use the pathspec struct interface
+ dir: convert fill_directory to use the pathspec struct interface
+ dir: remove struct path_simplify
+ mv: remove use of deprecated 'get_pathspec()'
(this branch is used by sb/pathspec-errors.)
Code clean-up in the pathspec API.
Will merge to 'master'.
* js/prepare-sequencer-more (2017-01-09) 38 commits
- sequencer (rebase -i): write out the final message
- sequencer (rebase -i): write the progress into files
- sequencer (rebase -i): show the progress
- sequencer (rebase -i): suggest --edit-todo upon unknown command
- sequencer (rebase -i): show only failed cherry-picks' output
- sequencer (rebase -i): show only failed `git commit`'s output
- sequencer: use run_command() directly
- sequencer: make reading author-script more elegant
- sequencer (rebase -i): differentiate between comments and 'noop'
- sequencer (rebase -i): implement the 'drop' command
- sequencer (rebase -i): allow rescheduling commands
- sequencer (rebase -i): respect strategy/strategy_opts settings
- sequencer (rebase -i): respect the rebase.autostash setting
- sequencer (rebase -i): run the post-rewrite hook, if needed
- sequencer (rebase -i): record interrupted commits in rewritten, too
- sequencer (rebase -i): copy commit notes at end
- sequencer (rebase -i): set the reflog message consistently
- sequencer (rebase -i): refactor setting the reflog message
- sequencer (rebase -i): allow fast-forwarding for edit/reword
- sequencer (rebase -i): implement the 'reword' command
- sequencer (rebase -i): leave a patch upon error
- sequencer (rebase -i): update refs after a successful rebase
- sequencer (rebase -i): the todo can be empty when continuing
- sequencer (rebase -i): skip some revert/cherry-pick specific code path
- sequencer (rebase -i): remove CHERRY_PICK_HEAD when no longer needed
- sequencer (rebase -i): allow continuing with staged changes
- sequencer (rebase -i): write an author-script file
- sequencer (rebase -i): implement the short commands
- sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
- sequencer (rebase -i): write the 'done' file
- sequencer (rebase -i): learn about the 'verbose' mode
- sequencer (rebase -i): implement the 'exec' command
- sequencer (rebase -i): implement the 'edit' command
- sequencer (rebase -i): implement the 'noop' command
- sequencer: support a new action: 'interactive rebase'
- sequencer: use a helper to find the commit message
- sequencer: move "else" keyword onto the same line as preceding brace
- sequencer: avoid unnecessary curly braces
The sequencer has further been extended in preparation to act as a
back-end for "rebase -i".
Waiting for review comments to be addressed.
* bw/realpath-wo-chdir (2017-01-09) 7 commits
(merged to 'next' on 2017-01-10 at ed315a40c8)
+ real_path: set errno when max number of symlinks is exceeded
+ real_path: prevent redefinition of MAXSYMLINKS
(merged to 'next' on 2016-12-22 at fea8fa870f)
+ real_path: canonicalize directory separators in root parts
+ real_path: have callers use real_pathdup and strbuf_realpath
+ real_path: create real_pathdup
+ real_path: convert real_path_internal to strbuf_realpath
+ real_path: resolve symlinks by hand
(this branch is tangled with bw/grep-recurse-submodules.)
The implementation of "real_path()" was to go there with chdir(2)
and call getcwd(3), but this obviously wouldn't be usable in a
threaded environment. Rewrite it to manually resolve relative
paths including symbolic links in path components.
Will merge to 'master'.
* js/difftool-builtin (2017-01-09) 5 commits
- t7800: run both builtin and scripted difftool, for now
- difftool: implement the functionality in the builtin
- difftool: add a skeleton for the upcoming builtin
- git_exec_path: do not return the result of getenv()
- git_exec_path: avoid Coverity warning about unfree()d result
Rewrite a scripted porcelain "git difftool" in C.
Expecting a reroll.
cf. <alpine.DEB.2.20.1701091228460.3469@virtualbox>
* sb/push-make-submodule-check-the-default (2016-11-29) 2 commits
(merged to 'next' on 2016-12-12 at 1863e05af5)
+ push: change submodule default to check when submodules exist
+ submodule add: extend force flag to add existing repos
Turn the default of "push.recurseSubmodules" to "check" when
submodules seem to be in use.
Will cook in 'next'.
* kn/ref-filter-branch-list (2017-01-10) 21 commits
- SQUASH???
- branch: implement '--format' option
- branch: use ref-filter printing APIs
- branch, tag: use porcelain output
- ref-filter: allow porcelain to translate messages in the output
- ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnames
- ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
- ref-filter: Do not abruptly die when using the 'lstrip=<N>' option
- ref-filter: rename the 'strip' option to 'lstrip'
- ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
- ref-filter: introduce refname_atom_parser()
- ref-filter: introduce refname_atom_parser_internal()
- ref-filter: make "%(symref)" atom work with the ':short' modifier
- ref-filter: add support for %(upstream:track,nobracket)
- ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
- ref-filter: introduce format_ref_array_item()
- ref-filter: move get_head_description() from branch.c
- ref-filter: modify "%(objectname:short)" to take length
- ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
- ref-filter: include reference to 'used_atom' within 'atom_value'
- ref-filter: implement %(if), %(then), and %(else) atoms
The code to list branches in "git branch" has been consolidated
with the more generic ref-filter API.
I think this is almost ready. Will wait for a few days, squash
fixes in if needed and merge to 'next'.
* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
(merged to 'next' on 2016-12-05 at 0c77e39cd5)
+ setup_git_env: avoid blind fall-back to ".git"
Originally merged to 'next' on 2016-10-26
This is the endgame of the topic to avoid blindly falling back to
".git" when the setup sequence said we are _not_ in Git repository.
A corner case that happens to work right now may be broken by a
call to die("BUG").
Will cook in 'next'.
* pb/bisect (2016-10-18) 27 commits
- bisect--helper: remove the dequote in bisect_start()
- bisect--helper: retire `--bisect-auto-next` subcommand
- bisect--helper: retire `--bisect-autostart` subcommand
- bisect--helper: retire `--bisect-write` subcommand
- bisect--helper: `bisect_replay` shell function in C
- bisect--helper: `bisect_log` shell function in C
- bisect--helper: retire `--write-terms` subcommand
- bisect--helper: retire `--check-expected-revs` subcommand
- bisect--helper: `bisect_state` & `bisect_head` shell function in C
- bisect--helper: `bisect_autostart` shell function in C
- bisect--helper: retire `--next-all` subcommand
- bisect--helper: retire `--bisect-clean-state` subcommand
- bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
- t6030: no cleanup with bad merge base
- bisect--helper: `bisect_start` shell function partially in C
- bisect--helper: `get_terms` & `bisect_terms` shell function in C
- bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: `check_and_set_terms` shell function in C
- bisect--helper: `bisect_write` shell function in C
- bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
- bisect--helper: `bisect_reset` shell function in C
- wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
- t6030: explicitly test for bisection cleanup
- bisect--helper: `bisect_clean_state` shell function in C
- bisect--helper: `write_terms` shell function in C
- bisect: rewrite `check_term_format` shell function in C
- bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
Move more parts of "git bisect" to C.
Expecting a reroll.
cf. <CAFZEwPPXPPHi8KiEGS9ggzNHDCGhuqMgH9Z8-Pf9GLshg8+LPA@mail.gmail.com>
cf. <CAFZEwPM9RSTGN54dzaw9gO9iZmsYjJ_d1SjUD4EzSDDbmh-XuA@mail.gmail.com>
* st/verify-tag (2016-10-10) 7 commits
- t/t7004-tag: Add --format specifier tests
- t/t7030-verify-tag: Add --format specifier tests
- builtin/tag: add --format argument for tag -v
- builtin/verify-tag: add --format to verify-tag
- tag: add format specifier to gpg_verify_tag
- ref-filter: add function to print single ref_array_item
- gpg-interface, tag: add GPG_VERIFY_QUIET flag
"git tag" and "git verify-tag" learned to put GPG verification
status in their "--format=<placeholders>" output format.
Waiting for a reroll.
cf. <20161007210721.20437-1-santiago@nyu.edu>
* sg/fix-versioncmp-with-common-suffix (2017-01-12) 8 commits
- versioncmp: generalize version sort suffix reordering
- versioncmp: factor out helper for suffix matching
- versioncmp: use earliest-longest contained suffix to determine sorting order
- versioncmp: cope with common part overlapping with prerelease suffix
- versioncmp: pass full tagnames to swap_prereleases()
- t7004-tag: add version sort tests to show prerelease reordering issues
- t7004-tag: use test_config helper
- t7004-tag: delete unnecessary tags with test_when_finished
The prereleaseSuffix feature of version comparison that is used in
"git tag -l" did not correctly when two or more prereleases for the
same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
are there and the code needs to compare 2.0-beta1 and 2.0-beta2).
Will merge to 'next'.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
(merged to 'next' on 2016-12-05 at 041946dae0)
+ merge: drop 'git merge <message> HEAD <commit>' syntax
Originally merged to 'next' on 2016-10-11
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
Will cook in 'next'.
--------------------------------------------------
[Discarded]
* sb/attr (2016-11-11) 35 commits
. completion: clone can initialize specific submodules
. clone: add --init-submodule=<pathspec> switch
. submodule update: add `--init-default-path` switch
. pathspec: allow escaped query values
. pathspec: allow querying for attributes
. pathspec: move prefix check out of the inner loop
. pathspec: move long magic parsing out of prefix_pathspec
. Documentation: fix a typo
. attr: keep attr stack for each check
. attr: convert to new threadsafe API
. attr: make git_check_attr_counted static
. attr.c: outline the future plans by heavily commenting
. attr.c: always pass check[] to collect_some_attrs()
. attr.c: introduce empty_attr_check_elems()
. attr.c: correct ugly hack for git_all_attrs()
. attr.c: rename a local variable check
. attr.c: pass struct git_attr_check down the callchain
. attr.c: add push_stack() helper
. attr: support quoting pathname patterns in C style
. attr: expose validity check for attribute names
. attr: add counted string version of git_check_attr()
. attr: retire git_check_attrs() API
. attr: convert git_check_attrs() callers to use the new API
. attr: convert git_all_attrs() to use "struct git_attr_check"
. attr: (re)introduce git_check_attr() and struct git_attr_check
. attr: rename function and struct related to checking attributes
. attr.c: plug small leak in parse_attr_line()
. attr.c: tighten constness around "git_attr" structure
. attr.c: simplify macroexpand_one()
. attr.c: mark where #if DEBUG ends more clearly
. attr.c: complete a sentence in a comment
. attr.c: explain the lack of attr-name syntax check in parse_attr()
. attr.c: update a stale comment on "struct match_attr"
. attr.c: use strchrnul() to scan for one line
. commit.c: use strchrnul() to scan for one line
The attributes API has been updated so that it can later be
optimized using the knowledge of which attributes are queried.
Building on top of the updated API, the pathspec machinery learned
to select only paths with given attributes set.
Superseded by bw/attr topic.
^ permalink raw reply
* Re: gitk pull request // was: Re: gitk: "lime" color incompatible with older Tk versions
From: Paul Mackerras @ 2017-01-16 3:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: David Aguilar, Stefan Beller, Andrew Janke, git@vger.kernel.org
In-Reply-To: <xmqq37gldp8g.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Sat, Jan 14, 2017 at 06:35:43PM -0800, Junio C Hamano wrote:
> David Aguilar <davvid@gmail.com> writes:
>
> > On Fri, Jan 13, 2017 at 03:20:43AM -0800, David Aguilar wrote:
> >>
> >> Ping.. it would be nice to get this patch applied.
> >
> > Sorry for the noise, and thank you Paul for the fix.
> > This was already fixed by Paul in gitk@22a713c72df.
> >
> > I'm sure Junio will merge gitk.git into git.git soon enough so I
> > can sit tight until then, but while I'm here I might as well
> > send out a pull request:
> >
> > The following changes since commit 22a713c72df8b6799c59287c50cee44c4a6db51e:
> >
> > gitk: Follow themed bgcolor in help dialogs (2016-03-19 14:12:21 +1100)
> >
> > are available in the git repository at:
> >
> > git://ozlabs.org/~paulus/gitk.git
> >
> > for you to fetch changes up to fbf426478e540f4737860dae622603cc0daba3d2:
> >
> > gitk: Update copyright notice to 2016 (2016-12-12 20:46:42 +1100)
>
> Pinging Paul to signal me that his tree is ready to pull from is
> appreciated, and asking Paul if his tree is ready to be pulled and
> then relaying his answer to me is also fine, but I am sensing that
> this message is neither. So let me double check.
>
> Paul, is it a good time to pull, or do you still have something not
> published yet that should go together with what you have already
> queued?
I recently pushed out one more commit to update the Russian
translation from Dimitriy Ryazantcev. The head is now 8fef3f36b779.
I have a couple more series that I am currently reviewing, but nothing
immediately ready to publish. It would be a good time for you to do a
pull, since the "lime" color fix and the memory consumption fixes
should be helpful for a lot of people.
Thanks,
Paul.
^ permalink raw reply
* Re: [RFC] stash --continue
From: Jacob Keller @ 2017-01-16 3:59 UTC (permalink / raw)
To: Stephan Beyer; +Cc: git
In-Reply-To: <cd784a4e-ee99-564e-81de-9f7f6cc26c67@gmx.net>
On Sun, Jan 15, 2017 at 3:56 PM, Stephan Beyer <s-beyer@gmx.net> wrote:
> Hi,
>
> a git-newbie-ish co-worker uses git-stash sometimes. Last time he used
> "git stash pop", he got into a merge conflict. After he resolved the
> conflict, he did not know what to do to get the repository into the
> wanted state. In his case, it was only "git add <resolved files>"
> followed by a "git reset" and a "git stash drop", but there may be more
> involved cases when your index is not clean before "git stash pop" and
> you want to have your index as before.
>
> This led to the idea to have something like "git stash --continue"[1]
> that would expect the user to "git add" the resolved files (as "git
> status" suggests) but then leads to the expected result, i.e. the index
> being the same as before the conflict, the stash being dropped (if "pop"
> was used instead of "apply"), etc.
>
> Likewise, some "git stash --abort"[2] might be useful in case you did
> "git stash pop" with the wrong stash in mind.
>
> What do you think about that?
>
This sounds like a useful extension to me.
Thanks,
Jake
^ permalink raw reply
* Re: [RFC for GIT] pull-request: add praise to people doing QA
From: Jacob Keller @ 2017-01-16 4:05 UTC (permalink / raw)
To: Junio C Hamano
Cc: Wolfram Sang, Git mailing list, linux-kernel@vger.kernel.org
In-Reply-To: <xmqqlgubc04z.fsf@gitster.mtv.corp.google.com>
On Sun, Jan 15, 2017 at 4:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> As to the implementation, I am wondering if we can make this somehow
> work well with the "trailers" code we already have, instead of
> inventing yet another parser of trailers.
>
> In its current shape, "interpret-trailers" focuses on "editing" an
> existing commit log message to tweak the trailer lines. That mode
> of operation would help amending and rebasing, and to do that it
> needs to parse the commit log message, identify trailer blocks,
> parse out each trailer lines, etc.
>
> There is no fundamental reason why its output must be an edited
> original commit log message---it should be usable as a filter that
> picks trailer lines of the selected trailer type, like "Tested-By",
> etc.
I have been looking at ways to use the interpret-trailers as a way to
filter commits and print out trailers, and this sort of feature would
be useful to me if it were generic. (and then pull-request could use
the generic interface to grab the data and then parse it into a praise
format)
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH v3 01/38] sequencer: avoid unnecessary curly braces
From: Jacob Keller @ 2017-01-16 4:10 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, Junio C Hamano, Git mailing list,
Kevin Daudt, Dennis Kaarsemaker, Stephan Beyer
In-Reply-To: <20170114180550.ebra5qexewetuoyk@sigill.intra.peff.net>
On Sat, Jan 14, 2017 at 10:05 AM, Jeff King <peff@peff.net> wrote:
> On Sat, Jan 14, 2017 at 06:57:13PM +0100, Johannes Schindelin wrote:
>
>> On Thu, 12 Jan 2017, Junio C Hamano wrote:
>>
>> > Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>> >
>> > >
>> > > - if (!commit->parents) {
>> > > + if (!commit->parents)
>> > > parent = NULL;
>> > > - }
>> > > else if (commit->parents->next) {
>> > > /* Reverting or cherry-picking a merge commit */
>> > > int cnt;
>> >
>> > The result becomes
>> >
>> > if (...)
>> > single statement;
>> > else if (...) {
>> > multiple;
>> > statements;
>> > }
>> >
>> > which is not quite an improvement.
>>
>> Yet, this used to be the coding style of Git, and your statement comes
>> quite as a surprise to me.
>
> Yeah, I thought we were OK with:
>
> if (cond)
> single statement;
> else {
> multiple;
> statements;
> }
>
> but not the other way around:
>
> if (cond) {
> multiple;
> statements;
> } else
> single statement;
>
> I don't know if the "else if" changes that or not, but I certainly have
> written things like your patch does.
>
> -Peff
Personally, I am of the faith that if any branch of the
if-then-else-if-then-else blocks needs braces, then all branches
should use braces. However, I think that
if (condition)
<line>
else {
<block>
}
is reasonably close to this, as the main part can still clearly see
the if condition pretty well.
Thanks,
Jake
^ permalink raw reply
* Different merges from translation perspective
From: Alexander Shopov @ 2017-01-16 4:41 UTC (permalink / raw)
To: Git List
Hi all,
What is the difference between simple, fast forward, automatic and
trivial merge?
I am updating the translation and the only thing I am sure about is
that these four are not octopus merges,
Fast forward is when current state is ancestor of tip, automatic merge
is when the merge algorithm is decided by git rather than developer.
What about simple (git-merge-octopus.sh) and trivial
(builtin/merge.c)?
Kind regards:
al_shopov
^ permalink raw reply
* Re: Different merges from translation perspective
From: Jacob Keller @ 2017-01-16 6:00 UTC (permalink / raw)
To: Alexander Shopov; +Cc: Git List
In-Reply-To: <CAP6f5MkOoDUqHCvLNQ+xJGWTbrdecet9W_JK5y7JeAnBpGeAaw@mail.gmail.com>
On Sun, Jan 15, 2017 at 8:41 PM, Alexander Shopov <ash@kambanaria.org> wrote:
> Hi all,
> What is the difference between simple, fast forward, automatic and
> trivial merge?
> I am updating the translation and the only thing I am sure about is
> that these four are not octopus merges,
> Fast forward is when current state is ancestor of tip, automatic merge
> is when the merge algorithm is decided by git rather than developer.
> What about simple (git-merge-octopus.sh) and trivial
> (builtin/merge.c)?
> Kind regards:
> al_shopov
Hi,
I'm not sure exactly what the documentation says regarding all of these things.
I know for sure that a fast-forward merge isn't "really" a merge in
the sense that no merge-commit is generated. Instead, the current
branch is simply fast-forwarded to the new result (since it's a direct
ancestor of the new tip.
I don't really have any answers for the others.
Thanks,
Jake
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Johannes Sixt @ 2017-01-16 6:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <xmqqh94zbwlu.fsf@gitster.mtv.corp.google.com>
Am 16.01.2017 um 02:51 schrieb Junio C Hamano:
> * jk/vreport-sanitize (2017-01-11) 2 commits
> - vreport: sanitize ASCII control chars
> - Revert "vreportf: avoid intermediate buffer"
>
> An error message with an ASCII control character like '\r' in it
> can alter the message to hide its early part, which is problematic
> when a remote side gives such an error message that the local side
> will relay with a "remote: " prefix.
>
> Will merge to 'next'.
Please be not too hasty with advancing this topic to master. I could
imagine that there is some unwanted fallout on systems where the
end-of-line marker CRLF is common. Though, I haven't tested the topic
myself, yet, nor do I expect regressions in *my* typical workflow.
-- Hannes
^ permalink raw reply
* Re: Different merges from translation perspective
From: Junio C Hamano @ 2017-01-16 7:44 UTC (permalink / raw)
To: Alexander Shopov; +Cc: Git List
In-Reply-To: <CAP6f5MkOoDUqHCvLNQ+xJGWTbrdecet9W_JK5y7JeAnBpGeAaw@mail.gmail.com>
Alexander Shopov <ash@kambanaria.org> writes:
> What is the difference between simple, fast forward, automatic and
> trivial merge?
"fast forward" and "trivial" (and "automatic" to some degree) are
technical terms with precise meaning. Other phrases that are
related to "merge" that are not in your list are "already
up-to-date" and "real merge".
I do not think "simple" is among these words, but I can see it used
colloquially to mean "a real merge that is easy to resolve",
e.g. "if you get conflicts that is not simple, you may have to study
what both sides did carefully before you can decide the correct
merge result".
When you are on commit X and trying to merge commit Y, various
things can happen.
* There is one case where nothing happens. You may have cloned
from another repository and the tip of the branch was at commit Y
back then---since then you built on it and you are now at commit
X, and nobody else did anything in that repository in the
meantime. You try to merge from there, and find that the tip of
the branch is still Y. Your history leading to X contains
everything the other side of the history leading to Y contains,
so there is no need to do anything.
We say that in this situation, your branch is already up-to-date.
* There is another case where no new commit is created. You may
have cloned like the above case and got commit X, and haven't
done anything since then, while others worked on the branch to
advance the tip to commit Y. You try to merge their work. Their
history leading to Y contains everything you have in your history
leading to X contains. The only thing we need is to "fast forward"
your tip of the branch from X to Y, and make the index and the
working tree to match.
We say that in this situation, your branch can be fast forwarded.
* All other cases, we need to come up with a new state that is the
result of merging X and Y, and record it as a child commit of
both X and Y, i.e. create a merge commit.
We say that this situation requires a real merge.
* When a real merge is needed, there are a few subcases.
- The two histories being merged may have changed their own set
of files, without overlap. Your commit X, since your history
diverged from the history that leads to commit Y, may have
worked only on the source file, while commit Y, since it
diverged from your history, may have worked only on the
documentation file, in a hypothetical two-file project. In
such a case, the copy of the documentation file you have at
commit X is in the state the file was in when the histories
diverged, and only the other side modified it, and we can take
the documentation file from commit Y (i.e. the other side) as
the result. Similarly, because only you changed the source
file while the other side didn't touch it, we take the source
file from commit X. We can come up with the merge result
without even inspecting the file contents.
The act of coming up with the result of the merge by pure
equality of the file contents (i.e. one side modified, the
other side left intact) is called "tree level merge", and a
merge, all of whose paths can be resolved by tree level merge,
is called "trivial merge".
- If a merge is not "trivial", we'd need to dig down to "file
level merge". The contents of a file that was touched by both
sides need to be computed.
- In a merge that requires "file level merge", you and the other
side may have modified the same file, but touched different and
non-overlapping parts. You may have updated text in section 1
of the documentation file while the other side may have fixed
typo in section 3. We use the same "three-way merge" principle
used in the "tree-level merge"; if you did not touch a part of
file that was modified by the other side, we take what the
other side did, and vice versa, to come up with the merge
result. There are other ways to mechanically come up with the
file level merge result that the user can supply (low level
merge drivers). The result of such a mechanical merge, when
successfully recorded, is often called "automatic merge".
^ permalink raw reply
* Re: gitk pull request // was: Re: gitk: "lime" color incompatible with older Tk versions
From: Junio C Hamano @ 2017-01-16 7:48 UTC (permalink / raw)
To: Paul Mackerras
Cc: David Aguilar, Stefan Beller, Andrew Janke, git@vger.kernel.org
In-Reply-To: <20170116031706.GA3322@fergus.ozlabs.ibm.com>
Paul Mackerras <paulus@ozlabs.org> writes:
>> Paul, is it a good time to pull, or do you still have something not
>> published yet that should go together with what you have already
>> queued?
>
> I recently pushed out one more commit to update the Russian
> translation from Dimitriy Ryazantcev. The head is now 8fef3f36b779.
> I have a couple more series that I am currently reviewing, but nothing
> immediately ready to publish. It would be a good time for you to do a
> pull, since the "lime" color fix and the memory consumption fixes
> should be helpful for a lot of people.
Thanks. I did want to get the memory consumption fix sooner rather
than later, and this is very much appreciated.
Pulled.
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Junio C Hamano @ 2017-01-16 7:48 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <257b4175-9879-7814-5d8d-02050792574d@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> Am 16.01.2017 um 02:51 schrieb Junio C Hamano:
>> * jk/vreport-sanitize (2017-01-11) 2 commits
>> - vreport: sanitize ASCII control chars
>> - Revert "vreportf: avoid intermediate buffer"
>>
>> An error message with an ASCII control character like '\r' in it
>> can alter the message to hide its early part, which is problematic
>> when a remote side gives such an error message that the local side
>> will relay with a "remote: " prefix.
>>
>> Will merge to 'next'.
>
> Please be not too hasty with advancing this topic to master. I could
> imagine that there is some unwanted fallout on systems where the
> end-of-line marker CRLF is common. Though, I haven't tested the topic
> myself, yet, nor do I expect regressions in *my* typical workflow.
Thanks; will wait for a further discussion on the topic's thread
then.
^ permalink raw reply
* Re: [PATCH 00/27] Revamp the attribute system; another round
From: Jeff King @ 2017-01-16 8:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brandon Williams, git, pclouds, sbeller
In-Reply-To: <xmqq37gjdgxn.fsf@gitster.mtv.corp.google.com>
On Sun, Jan 15, 2017 at 03:47:16PM -0800, Junio C Hamano wrote:
> This one unfortunately clashes with jk/nofollow-attr-ignore where
> Peff adds sanity to refuse following symbolic links when reading
> .gitignore and .gitattributes; I'll eject jk/nofollow-attr-ignore
> topic for now and see how well this topic fits together with the
> remainder of the topics in flight.
Yeah, that's a good plan. I think my re-roll of the nofollow stuff will
be pretty major, and may not end up touching the attribute code directly
at all.
-Peff
^ permalink raw reply
* Re: [RFC] stash: support filename argument
From: Marc Strapetz @ 2017-01-16 8:18 UTC (permalink / raw)
To: Stephan Beyer, Junio C Hamano, Thomas Gummerer; +Cc: git, kes-kes
In-Reply-To: <c0d46a97-b1c0-d9c9-e475-28e0368ac61f@gmx.net>
On 16.01.2017 01:41, Stephan Beyer wrote:
> Hi,
>
> On 01/16/2017 01:21 AM, Junio C Hamano wrote:
>> I haven't spent enough time to think if it even makes sense to
>> "stash" partially, leaving the working tree still dirty. My initial
>> reaction was "then stashing away the dirty WIP state to get a spiffy
>> clean working environment becomes impossible", and I still need time
>> to recover from that ;-) So as to the desirablity of this "feature",
>> I have no strong opinion for or against yet.
> I do remember that I simulated that feature a few times (either by
> adding the to-be-keep stuff (hunks, not only files) to the index and use
> --keep-index, and sometimes by making a temporary commit (to make sure
> to not lose anything) and then stash). So I think there is a valid
> desire of the feature.
I can confirm this from a GUI client perspective, for which this feature
makes probably even more sense than for command line. It has been
requested by our users quite often compared to other features and
compared to "git stash -p" support.
-Marc
^ permalink raw reply
* Re: [PATCH] Documentation/bisect: improve on (bad|new) and (good|bad)
From: Matthieu Moy @ 2017-01-16 9:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Couder, git, Manuel Ullmann, Christian Couder
In-Reply-To: <xmqqinpihiwz.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> The following part of the description:
>>
>> git bisect (bad|new) [<rev>]
>> git bisect (good|old) [<rev>...]
>>
>> may be a bit confusing, as a reader may wonder if instead it should be:
>>
>> git bisect (bad|good) [<rev>]
>> git bisect (old|new) [<rev>...]
>>
>> Of course the difference between "[<rev>]" and "[<rev>...]" should hint
>> that there is a good reason for the way it is.
>>
>> But we can further clarify and complete the description by adding
>> "<term-new>" and "<term-old>" to the "bad|new" and "good|old"
>> alternatives.
>>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>> Documentation/git-bisect.txt | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Thanks. The patch looks good.
Looks good to me too.
> I think the answer to the question "why do we think we need a single
> bisect/bad?" is "because bisection is about assuming that there is
> only one commit that flips the tree state from 'old' to 'new' and
> finding that single commit".
I wouldn't say it's about "assuming" there's only one commit, but it's
about finding *one* such commit, i.e. it works if there are several such
commits, but won't find them all.
> But what if bad-A and bad-B have more than one merge bases? We
> won't know which side the badness came from.
>
> o---o---o---bad-A
> / \ /
> -----Good---o---o---o /
> \ / \
> o---o---o---bad-B
>
> Being able to bisect the region of DAG bound by "^Good bad-A bad-B"
> may have value in such a case. I dunno.
I could help finding several guilty commits, but anyway you can't
guarantee you'll find them all as soon as you use a binary search: if
the history looks like
--- Good --- Bad --- Good --- Good --- Bad --- Good --- Bad
then without examining all commits, you can't tell how many good->bad
switches occured.
But keeping several bad commits wouldn't help keeping the set of
potentially guilty commits small: bad commits appear on the positive
side in "^Good bad-A bad-B", so having more bad commits mean having a
larger DAG to explore (which is a bit counter-intuitive: without
thinking about it I'd have said "more info => less commits to explore").
So, if finding all guilty commits is not possible, I'm not sure how
valuable it is to try to find several of them.
OTOH, keeping several good commits is needed to find a commit for which
all parents are good and the commit is bad, i.e. distinguish
Good
\
Bad <-- this is the one.
/
Good
and
Good
\
Bad <-- need to dig further
/
Bad
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH v3 00/38] Teach the sequencer to act as rebase -i's backend
From: Johannes Schindelin @ 2017-01-16 10:45 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Kevin Daudt, Dennis Kaarsemaker, Stephan Beyer, Jeff King
In-Reply-To: <xmqqinpgdass.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Sat, 14 Jan 2017, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > You stated elsewhere that converting a script into a builtin should focus
> > on a faithful conversion.
> >
> > The original code is:
> >
> > . "$author_script"
>
> [...]
>
> If the code in the sequencer.c reads things other than the three
> variables we ourselves set, and make them into environment variables
> and propagate to subprocesses (hooks and editors), it would be a
> bug. The original did not intend to do that (the dot-sourcing is
> overly loose than reading three known variables and nothing else,
> but is OK because we do not support the case where end users muck
> with the file). Also, writing FOO=BAR alone (not "export FOO=BAR"
> or "FOO=BAR; export FOO") to the file wouldn't have exported FOO to
> subprocesses anyway.
That analysis cannot be completely correct, as the GIT_AUTHOR_* variables
*are* used by the `git commit` subprocess.
In any case, it is clear that we (I include all reviewers here) messed
this patch series up quite a bit. Hence I will be more careful from now on
to only act on suggestions that do, in fact, improve the patch series from
my point of view.
Ciao,
Johannes
^ permalink raw reply
* Re: [RFC] stash: support filename argument
From: Johannes Schindelin @ 2017-01-16 10:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Gummerer, git, kes-kes
In-Reply-To: <xmqqvatfc0rt.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Sun, 15 Jan 2017, Junio C Hamano wrote:
> I haven't spent enough time to think if it even makes sense to
> "stash" partially, leaving the working tree still dirty.
Think no more! We already support that with --keep-index, and it is a very
useful feature.
Ciao,
Johannes
^ permalink raw reply
* Re: [RFC] stash --continue
From: Johannes Schindelin @ 2017-01-16 10:54 UTC (permalink / raw)
To: Stephan Beyer; +Cc: git
In-Reply-To: <cd784a4e-ee99-564e-81de-9f7f6cc26c67@gmx.net>
Hi Stephan,
On Mon, 16 Jan 2017, Stephan Beyer wrote:
> a git-newbie-ish co-worker uses git-stash sometimes. Last time he used
> "git stash pop", he got into a merge conflict. After he resolved the
> conflict, he did not know what to do to get the repository into the
> wanted state. In his case, it was only "git add <resolved files>"
> followed by a "git reset" and a "git stash drop", but there may be more
> involved cases when your index is not clean before "git stash pop" and
> you want to have your index as before.
>
> This led to the idea to have something like "git stash --continue"[1]
More like "git stash pop --continue". Without the "pop" command, it does
not make too much sense.
Ciao,
Johannes
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Johannes Schindelin @ 2017-01-16 11:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqh94zbwlu.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Sun, 15 Jan 2017, Junio C Hamano wrote:
> * js/prepare-sequencer-more (2017-01-09) 38 commits
I think that it adds confusion rather than value to specifically use a
different branch name than I indicated in my submission, unless there is a
really good reason to do so (which I fail to see here).
> - sequencer (rebase -i): write out the final message
> - sequencer (rebase -i): write the progress into files
> - sequencer (rebase -i): show the progress
> - sequencer (rebase -i): suggest --edit-todo upon unknown command
> - sequencer (rebase -i): show only failed cherry-picks' output
> - sequencer (rebase -i): show only failed `git commit`'s output
> - sequencer: use run_command() directly
> - sequencer: make reading author-script more elegant
> - sequencer (rebase -i): differentiate between comments and 'noop'
> - sequencer (rebase -i): implement the 'drop' command
> - sequencer (rebase -i): allow rescheduling commands
> - sequencer (rebase -i): respect strategy/strategy_opts settings
> - sequencer (rebase -i): respect the rebase.autostash setting
> - sequencer (rebase -i): run the post-rewrite hook, if needed
> - sequencer (rebase -i): record interrupted commits in rewritten, too
> - sequencer (rebase -i): copy commit notes at end
> - sequencer (rebase -i): set the reflog message consistently
> - sequencer (rebase -i): refactor setting the reflog message
> - sequencer (rebase -i): allow fast-forwarding for edit/reword
> - sequencer (rebase -i): implement the 'reword' command
> - sequencer (rebase -i): leave a patch upon error
> - sequencer (rebase -i): update refs after a successful rebase
> - sequencer (rebase -i): the todo can be empty when continuing
> - sequencer (rebase -i): skip some revert/cherry-pick specific code path
> - sequencer (rebase -i): remove CHERRY_PICK_HEAD when no longer needed
> - sequencer (rebase -i): allow continuing with staged changes
> - sequencer (rebase -i): write an author-script file
> - sequencer (rebase -i): implement the short commands
> - sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
> - sequencer (rebase -i): write the 'done' file
> - sequencer (rebase -i): learn about the 'verbose' mode
> - sequencer (rebase -i): implement the 'exec' command
> - sequencer (rebase -i): implement the 'edit' command
> - sequencer (rebase -i): implement the 'noop' command
> - sequencer: support a new action: 'interactive rebase'
> - sequencer: use a helper to find the commit message
> - sequencer: move "else" keyword onto the same line as preceding brace
> - sequencer: avoid unnecessary curly braces
>
> The sequencer has further been extended in preparation to act as a
> back-end for "rebase -i".
>
> Waiting for review comments to be addressed.
The only outstanding review comments I know about are your objection to
the name of the read_env_script() function (which I shot down as bogus),
and the rather real bug fix I sent out as a fixup! which you may want to
squash in (in the alternative, I can mailbomb v4 of the entire sequencer-i
patch series, that is your choice).
Ciao,
Johannes
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Johannes Schindelin @ 2017-01-16 11:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqh94zbwlu.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Sun, 15 Jan 2017, Junio C Hamano wrote:
> Here are the topics that have been cooking. Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'. The ones marked with '.' do not appear in any of
> the integration branches, but I am still holding onto them.
A suggestion: since it is very, very tedious to find the latest
iteration's thread, as well as the corresponding commit in `pu` (or
whatever other branch you use), and since you auto-generate these lists
anyway, it would make things less cumbersome if the commit names of the
tips, as well as the Message-IDs of the cover-letter (or first patch) were
displayed with the topic branches.
It still would not address e.g. the problem that the original authors are
not notified about the current state of their submission by these What's
Cooking mails.
But it would improve the situation.
> * js/difftool-builtin (2017-01-09) 5 commits
> - t7800: run both builtin and scripted difftool, for now
> - difftool: implement the functionality in the builtin
> - difftool: add a skeleton for the upcoming builtin
> - git_exec_path: do not return the result of getenv()
This patch was not in my patch submission. Sneaking it into this topic
branch is not incorrect.
It does not matter, though, as I will drop the Coverity patches from this
patch series: the conflation of Coverity fixes with the builtin difftool
was a mistake, and I will thereby address that mistake.
Ciao,
Johannes
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Johannes Schindelin @ 2017-01-16 12:52 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, git, Jeff King
In-Reply-To: <257b4175-9879-7814-5d8d-02050792574d@kdbg.org>
Hi Hannes,
On Mon, 16 Jan 2017, Johannes Sixt wrote:
> Am 16.01.2017 um 02:51 schrieb Junio C Hamano:
> > * jk/vreport-sanitize (2017-01-11) 2 commits
> > - vreport: sanitize ASCII control chars
> > - Revert "vreportf: avoid intermediate buffer"
> >
> > An error message with an ASCII control character like '\r' in it
> > can alter the message to hide its early part, which is problematic
> > when a remote side gives such an error message that the local side
> > will relay with a "remote: " prefix.
> >
> > Will merge to 'next'.
>
> Please be not too hasty with advancing this topic to master. I could imagine
> that there is some unwanted fallout on systems where the end-of-line marker
> CRLF is common. Though, I haven't tested the topic myself, yet, nor do I
> expect regressions in *my* typical workflow.
Thank you for being so attentive.
This topic branch would indeed have caused problems. Worse: it would have
caused problems that are not covered by our test suite, as Git for
Windows' own utilities do not generate CR/LF line endings. So this
regression would have bit our users. Nasty.
Something like this is necessary, at least:
-- snipsnap --
Subject: [PATCH] fixup! vreport: sanitize ASCII control chars
The original patch is incorrect, as it turns carriage returns into
question marks.
However, carriage returns should be left alone when preceding a line feed,
and simply turned into line feeds otherwise.
To make the end result more readable, the logic is inverted so that the
common case (no substitution) is handled first.
While at it, let's lose the unnecessary curly braces.
We also add a regression test that verifies that carriage returns are
handled properly. And as we expect CR/LF to be handled properly also on
platforms other than Windows, this test case is not guarded by the MINGW
prerequisite.
Spotted by Hannes Sixt.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t0000-basic.sh | 8 ++++++++
usage.c | 9 ++++++---
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 60811a3a7c..d494cb7c05 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -1063,4 +1063,12 @@ test_expect_success 'very long name in the index handled sanely' '
test $len = 4098
'
+# This test verifies that the error reporting functions handle CR correctly.
+# --abbrev-ref is simply used out of convenience, as it reports an error including
+# a user-specified argument.
+test_expect_success 'error messages treat CR/LF correctly' '
+ test_must_fail git rev-parse --abbrev-ref "$(printf "CR/LF\\r\\n")" 2>err &&
+ grep "^fatal: .*CR/LF$" err
+'
+
test_done
diff --git a/usage.c b/usage.c
index 50a6ccee44..71bc7c0329 100644
--- a/usage.c
+++ b/usage.c
@@ -15,10 +15,13 @@ void vreportf(const char *prefix, const char *err, va_list params)
char *p;
vsnprintf(msg, sizeof(msg), err, params);
- for (p = msg; *p; p++) {
- if (iscntrl(*p) && *p != '\t' && *p != '\n')
+ for (p = msg; *p; p++)
+ if (!iscntrl(*p) || *p == '\t' || *p == '\n')
+ continue;
+ else if (*p != '\r')
*p = '?';
- }
+ else if (p[1] != '\n')
+ *p = '\n';
fprintf(fh, "%s%s\n", prefix, msg);
}
--
2.11.0.windows.3
^ permalink raw reply related
* Re: [RFC] stash: support filename argument
From: Johannes Schindelin @ 2017-01-16 13:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Gummerer, git, kes-kes
In-Reply-To: <alpine.DEB.2.20.1701161150320.3469@virtualbox>
Hi,
On Mon, 16 Jan 2017, Johannes Schindelin wrote:
> On Sun, 15 Jan 2017, Junio C Hamano wrote:
>
> > I haven't spent enough time to think if it even makes sense to
> > "stash" partially, leaving the working tree still dirty.
>
> Think no more! We already support that with --keep-index, and it is a very
> useful feature.
And of course there is `git stash -p`, which is also very useful,
*because* it leaves the working tree still dirty, in the desired way.
Ciao,
Johannes
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Jeff King @ 2017-01-16 16:04 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johannes Sixt, Junio C Hamano, git
In-Reply-To: <alpine.DEB.2.20.1701161251100.3469@virtualbox>
On Mon, Jan 16, 2017 at 01:52:39PM +0100, Johannes Schindelin wrote:
> > > An error message with an ASCII control character like '\r' in it
> > > can alter the message to hide its early part, which is problematic
> > > when a remote side gives such an error message that the local side
> > > will relay with a "remote: " prefix.
> > >
> > > Will merge to 'next'.
> >
> > Please be not too hasty with advancing this topic to master. I could imagine
> > that there is some unwanted fallout on systems where the end-of-line marker
> > CRLF is common. Though, I haven't tested the topic myself, yet, nor do I
> > expect regressions in *my* typical workflow.
>
> Thank you for being so attentive.
>
> This topic branch would indeed have caused problems. Worse: it would have
> caused problems that are not covered by our test suite, as Git for
> Windows' own utilities do not generate CR/LF line endings. So this
> regression would have bit our users. Nasty.
Hmm. I am not sure to what degree CRLFs are actually a problem here.
Keep in mind these are error messages generated via error(), and so not
processing arbitrary data. I can imagine that CRs might come from:
1. A parameter like a filename or revision name. If I ask for a
rev-parse of "foo\r\n", then it's probably useful to mention the
"\r" in the error message, rather than ignoring it (or converting
it to a line-feed).
And I think that would apply to any input parameter we show via
error(), etc, if it is connected to a newline (ideally we would
show newlines as "?", too, but we cannot tell the difference
between ones from parameters, and ones that are part of the error
message).
2. The printf-fmt strings themselves. But these come from C code,
which just uses "\n". My impression is that it is fprintf() which
is responsible for converting that to "\r\n". And we are doing our
sanitizing here between an snprintf(), and an fprintf() of the
result. So it should see only the raw "\n" fields.
I am certainly open to loosening the sanitizing for CR to make things
work seamlessly on Windows. But I am having trouble imagining a case
that is actually negatively impacted.
> -- snipsnap --
> Subject: [PATCH] fixup! vreport: sanitize ASCII control chars
Given the subtlety here, I'd much rather have a patch on top.
> The original patch is incorrect, as it turns carriage returns into
> question marks.
>
> However, carriage returns should be left alone when preceding a line feed,
> and simply turned into line feeds otherwise.
The question of whether to leave CRLFs alone is addressed above. But I
do not understand why you'd want a lone CR to be converted to a line
feed. Running:
git rev-parse --abbrev-ref "$(printf "foo\r")"
with my patch gets you:
fatal: ambiguous argument 'foo?': unknown revision or path not in the working tree.
But with yours:
fatal: ambiguous argument 'foo
': unknown revision or path not in the working tree.
Obviously the "?" thing is a lossy transformation, but I do not see how
a newline is any less confusing (but see below for some thoughts).
> To make the end result more readable, the logic is inverted so that the
> common case (no substitution) is handled first.
>
> While at it, let's lose the unnecessary curly braces.
Please don't. Obviously C treats the "if/else" as a single unit, but
IMHO it's less error-prone to include the braces any time there are
multiple visual lines. E.g., something like:
while (foo)
if (bar)
one();
else
two();
three();
is much easier to spot as wrong when you would require braces either
way (and not relevant here, but I'd say that even an inner block with a
comment deserves braces for the same reason).
> We also add a regression test that verifies that carriage returns are
> handled properly. And as we expect CR/LF to be handled properly also on
> platforms other than Windows, this test case is not guarded by the MINGW
> prerequisite.
I am not sure what "properly" means here. In your test:
> +# This test verifies that the error reporting functions handle CR correctly.
> +# --abbrev-ref is simply used out of convenience, as it reports an error including
> +# a user-specified argument.
> +test_expect_success 'error messages treat CR/LF correctly' '
> + test_must_fail git rev-parse --abbrev-ref "$(printf "CR/LF\\r\\n")" 2>err &&
> + grep "^fatal: .*CR/LF$" err
> +'
The "\n" is eaten by the shell, and git sees only "CR/LF\r". So we are
not testing the CRLF case in vreportf() at all.
We do end up with "CR/LF\n" in vreportf(), which is presumably converted
by fprintf() to "CR/LF\r\n" on Windows. And so perhaps that is why you
are doing the "convert \r to \n" thing above.
But I still think it's not doing the right thing. Git _didn't_ see CRLF,
it saw CR.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Johannes Schindelin @ 2017-01-16 17:06 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Junio C Hamano, git
In-Reply-To: <20170116160456.ltbb7ofe47xos7xo@sigill.intra.peff.net>
Hi Peff,
On Mon, 16 Jan 2017, Jeff King wrote:
> On Mon, Jan 16, 2017 at 01:52:39PM +0100, Johannes Schindelin wrote:
>
> > > > An error message with an ASCII control character like '\r' in it
> > > > can alter the message to hide its early part, which is problematic
> > > > when a remote side gives such an error message that the local side
> > > > will relay with a "remote: " prefix.
> > > >
> > > > Will merge to 'next'.
> > >
> > > Please be not too hasty with advancing this topic to master. I could imagine
> > > that there is some unwanted fallout on systems where the end-of-line marker
> > > CRLF is common. Though, I haven't tested the topic myself, yet, nor do I
> > > expect regressions in *my* typical workflow.
> >
> > Thank you for being so attentive.
> >
> > This topic branch would indeed have caused problems. Worse: it would have
> > caused problems that are not covered by our test suite, as Git for
> > Windows' own utilities do not generate CR/LF line endings. So this
> > regression would have bit our users. Nasty.
>
> Hmm. I am not sure to what degree CRLFs are actually a problem here.
> Keep in mind these are error messages generated via error(), and so not
> processing arbitrary data. I can imagine that CRs might come from:
Please note the regression test I added. It uses rev-parse's --abbrev-ref
option which quotes the argument when erroring out. This argument then
gets munged.
So error() (or in this case, die()) *very much* processes arbitrary data.
I *know* that rev-parse --abbrev-ref is an artificial example, it is
highly unlikely that anybody will use
git rev-parse --abbrev-ref "$(<call an external program here that
generates CR/LF line endings>)"
However, there are plenty other cases in regular Git usage where arguments
are generated by external programs to which we have no business dictating a
specific line ending style.
If you absolutely insist, I will spend time to find a plausible example
and use that in the regression test. However, that would not really
improve anything, as the purpose of the regression test is simply to
demonstrate that a user-provided argument's CR/LF does not get munged
incorrectly. And the test I provided serves that purpose perfectly.
> 1. A parameter like a filename or revision name. If I ask for a
> rev-parse of "foo\r\n", then it's probably useful to mention the
> "\r" in the error message, rather than ignoring it (or converting
> it to a line-feed).
>
> And I think that would apply to any input parameter we show via
> error(), etc, if it is connected to a newline (ideally we would
> show newlines as "?", too, but we cannot tell the difference
> between ones from parameters, and ones that are part of the error
> message).
I think it is doing users a really great disservice to munge up CR or LF
into question marks. I *guarantee* you that it confuses users. And not
because they are dumb, but because the code violates the Law of Least
Surprise.
> I am certainly open to loosening the sanitizing for CR to make things
> work seamlessly on Windows. But I am having trouble imagining a case
> that is actually negatively impacted.
Git accepts all kinds of arguments, not just file names. It is totally
legitimate, and you probably can show use cases of that yourself, to
generate those arguments by other programs.
These programs can generate CR/LF line endings.
While well-intentioned, your changes could make things even unreadable.
Certainly confusing.
> > -- snipsnap --
> > Subject: [PATCH] fixup! vreport: sanitize ASCII control chars
>
> Given the subtlety here, I'd much rather have a patch on top.
Fine.
> > The original patch is incorrect, as it turns carriage returns into
> > question marks.
> >
> > However, carriage returns should be left alone when preceding a line feed,
> > and simply turned into line feeds otherwise.
>
> The question of whether to leave CRLFs alone is addressed above. But I
> do not understand why you'd want a lone CR to be converted to a line
> feed. Running:
>
> git rev-parse --abbrev-ref "$(printf "foo\r")"
>
> with my patch gets you:
>
> fatal: ambiguous argument 'foo?': unknown revision or path not in the working tree.
>
> But with yours:
>
> fatal: ambiguous argument 'foo
> ': unknown revision or path not in the working tree.
>
> Obviously the "?" thing is a lossy transformation,
s/lossy/confusing/
Probably not to you, because you came up with the transformation, but to
literally everybody else.
> but I do not see how a newline is any less confusing (but see below for
> some thoughts).
The more common bug report to be expected would show that multi-line
arguments are converted to ending in question marks. That is not the user
experience for which I am aiming.
> > To make the end result more readable, the logic is inverted so that the
> > common case (no substitution) is handled first.
> >
> > While at it, let's lose the unnecessary curly braces.
>
> Please don't. Obviously C treats the "if/else" as a single unit, but
> IMHO it's less error-prone to include the braces any time there are
> multiple visual lines. E.g., something like:
>
> while (foo)
> if (bar)
> one();
> else
> two();
> three();
>
> is much easier to spot as wrong when you would require braces either
> way (and not relevant here, but I'd say that even an inner block with a
> comment deserves braces for the same reason).
There is no documentation about the preferred coding style.
For years, I saw patches, and provided patches myself, that *avoided*
curly braces when not necessary (in addition to aiming for shorter arms to
come before longer arms in conditionals).
Now all of a sudden Junio *and* you suggest unnecessary curly braces to be
added.
That is inconsistent at best, and confusing. Maybe you two gentle people
can make up your mind and document the final verdict, and for extra
brownie points automate the formatting so that patch reviews are not
dominated by coding style comments that would be better addressed by
machines than by humans.
> > We also add a regression test that verifies that carriage returns are
> > handled properly. And as we expect CR/LF to be handled properly also on
> > platforms other than Windows, this test case is not guarded by the MINGW
> > prerequisite.
>
> I am not sure what "properly" means here. In your test:
>
> > +# This test verifies that the error reporting functions handle CR correctly.
> > +# --abbrev-ref is simply used out of convenience, as it reports an error including
> > +# a user-specified argument.
> > +test_expect_success 'error messages treat CR/LF correctly' '
> > + test_must_fail git rev-parse --abbrev-ref "$(printf "CR/LF\\r\\n")" 2>err &&
> > + grep "^fatal: .*CR/LF$" err
> > +'
>
> The "\n" is eaten by the shell, and git sees only "CR/LF\r". So we are
> not testing the CRLF case in vreportf() at all.
True. Should be easy to fix, starting from my patch.
> We do end up with "CR/LF\n" in vreportf(), which is presumably converted
> by fprintf() to "CR/LF\r\n" on Windows. And so perhaps that is why you
> are doing the "convert \r to \n" thing above.
>
> But I still think it's not doing the right thing. Git _didn't_ see CRLF,
> it saw CR.
You know, I don't care. As long as carriage returns are either left alone
(which conflicts specifically with your stated goal) or at least are
handled gracefully when coming before line feeds.
Converting stray carriage returns to question marks is confusing, and of
course I would take the brunt of the bug reports, so I do not look
favorably on that change.
My patch was just a starter, to help with fixing the patch series before
it hits `next`.
Ciao,
Johannes
^ permalink raw reply
* [PATCH 2/2] configure.ac: Fix --without-iconv
From: Bernd Kuhls @ 2017-01-16 19:56 UTC (permalink / raw)
To: git; +Cc: Bernd Kuhls
In-Reply-To: <20170116195638.3713-1-bernd.kuhls@writeme.com>
GIT_PARSE_WITH(iconv)) sets NO_ICONV=YesPlease in
https://github.com/git/git/blob/maint/configure.ac#L327
But the command GIT_CONF_SUBST([NO_ICONV]) in
https://github.com/git/git/blob/maint/configure.ac#L618
is only executed when NO_ICONV is an empty variable
https://github.com/git/git/blob/maint/configure.ac#L578
which has the effect that NO_ICONV=YesPlease is not written to
config.mak.autogen which breaks compilation in systems without iconv.
Signed-off-by: Bernd Kuhls <bernd.kuhls@writeme.com>
---
configure.ac | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac
index 63e71a472..419469315 100644
--- a/configure.ac
+++ b/configure.ac
@@ -614,15 +614,15 @@ LIBS="$old_LIBS"
GIT_UNSTASH_FLAGS($ICONVDIR)
-GIT_CONF_SUBST([NEEDS_LIBICONV])
-GIT_CONF_SUBST([NO_ICONV])
-
if test -n "$NO_ICONV"; then
NEEDS_LIBICONV=
fi
fi
+GIT_CONF_SUBST([NEEDS_LIBICONV])
+GIT_CONF_SUBST([NO_ICONV])
+
#
# Define NO_DEFLATE_BOUND if deflateBound is missing from zlib.
--
2.11.0
^ permalink raw reply related
* [PATCH 1/2] configure.ac: fix old iconv check
From: Bernd Kuhls @ 2017-01-16 19:56 UTC (permalink / raw)
To: git; +Cc: Bernd Kuhls
According to
https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Running-the-Compiler.html
the parameter syntax of AC_COMPILE_IFELSE is
(input, [action-if-true], [action-if-false])
Displaying "no" when the test was positive and enabling support for old
iconv implementations by OLD_ICONV=UnfortunatelyYes when the test fails
it obviously wrong. This patch switches the actions to fix the problem.
Signed-off-by: Bernd Kuhls <bernd.kuhls@writeme.com>
---
configure.ac | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index 0b15f04b1..63e71a472 100644
--- a/configure.ac
+++ b/configure.ac
@@ -759,9 +759,9 @@ GIT_STASH_FLAGS($ICONVDIR)
AC_MSG_CHECKING([for old iconv()])
AC_COMPILE_IFELSE([OLDICONVTEST_SRC],
- [AC_MSG_RESULT([no])],
[AC_MSG_RESULT([yes])
- OLD_ICONV=UnfortunatelyYes])
+ OLD_ICONV=UnfortunatelyYes],
+ [AC_MSG_RESULT([no])])
GIT_UNSTASH_FLAGS($ICONVDIR)
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox