* Re: [PATCH 1/2] ls-files: adding support for submodules
From: Junio C Hamano @ 2016-09-27 22:23 UTC (permalink / raw)
To: Brandon Williams; +Cc: Jeff King, git
In-Reply-To: <20160927220955.GA38615@google.com>
Brandon Williams <bmwill@google.com> writes:
>> If you are in a subdirectory of your superproject, say, a/,
>>
>> cd a && git ls-files --recurse-submodules -- "b*"
>>
>> I would expect we would recurse into the submodule at "a/b" and find
>> "b/file-at-top-of-B". What does the internal invocation to do so
>> would look like? I would think "git -C b --super=b ls-files" that
>> is run from "a".
Actually, the internal invocation may have to be
$ git --super=a/b ls-files -- "a/b*"
if the desired overall output needs to be in the "--full-name" mode.
That is, the top-level recursive one may be
cd a && git ls-files --recurse-submodules --full-name -- "b*"
This top-level "ls-files" will have "prefix" set to "a/". Because
it is run in the "--full-name" mode, after finding that the
submodule at "a/b" matches the given pathspec and deciding to
recurse into it, it needs to arrange that paths stored in the index
of the submodule are prefixed with "a/b/", not with "b/", when
shown.
^ permalink raw reply
* Re: thoughts on error passing, was Re: [PATCH 2/2] fsck: handle bad trees like other errors
From: David Turner @ 2016-09-27 22:57 UTC (permalink / raw)
To: Jeff King; +Cc: git, mhagger, David Turner
In-Reply-To: <20160927191955.mympqgylrxhkp24n@sigill.intra.peff.net>
On Tue, 2016-09-27 at 15:19 -0400, Jeff King wrote:
> On Tue, Sep 27, 2016 at 11:19:34AM -0400, David Turner wrote:
>
> > > typedef void (*err_fn)(const char *, ...);
> > >
> > > static int decode_tree_entry(struct tree_desc *desc,
> > > const char *buf, unsigned long size,
> > > err_fn err)
> > > {
> > > ...
> > > if (size < 23 || buf[size - 21]) {
> > > err("too-short tree object");
> > > return -1;
> > > }
> > > }
> > >
> > > I dunno. Maybe that is overengineering. I guess we only hit the strbufs
> > > in the error path (which used to die!), so nobody really cares that much
> > > about the extra allocation.
> >
> > I don't really like err_fn because:
> > (a) without a baton, it's somewhat less general (or less thread-safe)
> > than the strbuf approach and
> > (b) with a baton, it's two arguments instead of one.
>
> I'm going to ramble for a minute, and I don't think it's worth exploring
> for this patch series in particular, so feel free to ignore me.
>
> I think this error concept could be extended fairly elegantly with
> something like:
>
> typedef void (*err_fn)(void *, const char *fmt, va_list ap)
> struct error_context {
> err_fn fn;
> void *data;
> };
>
> int report_error(struct error_context *err, const char *fmt, ...)
> {
> if (err->fn) {
> va_list ap;
> va_start(ap, fmt);
> err->fn(err->data, fmt, ap);
> va_end(ap);
> }
> return -1;
> }
>
> Then low-level functions just take a context and do:
>
> return report_error(&err, "some error: %s", foo);
>
> And then the callers would pick one of a few generic error contexts:
>
> - passing NULL silences the errors
Overall, +1.
I guess I would rather have a sentinel value for silencing errors,
because I'm worried that someone might read NULL as "don't handle the
errors, just die". Of course, code review would hopefully catch this,
but even so, it would be easier to read foo(x, y, silence_errors) than
foo(x, y, null).
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2016, #07; Fri, 23)
From: Junio C Hamano @ 2016-09-27 23:11 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1609242101100.129229@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> In your previous kitchen status ("What's cooking") you hinted at a
> possible v2.10.1 soon. I have a couple of bugfixes lined up for Git for
> Windows and would like to avoid unnecessarily frequent release
> engineering... Any more concrete ideas on a date for this version?
I scanned RelNotes for 2.11 and identified these topics that we'd
want to have in 'maint'.
bw/pathspec-remove-unused-extern-decl # 1 (6 days ago)
rs/checkout-some-states-are-const # 1 (6 days ago)
rs/strbuf-remove-fix # 1 (6 days ago)
rs/unpack-trees-reduce-file-scope-global # 1 (6 days ago)
mr/vcs-svn-printf-ulong # 1 (6 days ago)
sy/git-gui-i18n-ja # 7 (12 days ago)
jk/fix-remote-curl-url-wo-proto # 1 (12 days ago)
js/git-gui-commit-gpgsign # 2 (12 days ago)
jk/patch-ids-no-merges # 2 (6 days ago)
ew/http-do-not-forget-to-call-curl-multi-remove-handle # 3 (6 days ago)
rs/xdiff-merge-overlapping-hunks-for-W-context # 1 (6 days ago)
ks/perf-build-with-autoconf # 1 (6 days ago)
jt/format-patch-base-info-above-sig # 1 (6 days ago)
jk/rebase-i-drop-ident-check # 1 (6 days ago)
jk/reduce-gc-aggressive-depth # 1 (6 days ago)
et/add-chmod-x # 1 (6 days ago)
tg/add-chmod+x-fix # 7 (24 hours ago)
Most are internal clean-ups that I do not mind leaving out, but I
think we want to have that "add --chmod=+x" fix in. As it hasn't
been enough time passed since the topic was merged to 'master', I'd
say either
(1) 2.10.1 with everything other than the last two in a few days
and 2.10.2 late next week with "add --chmod=+x" fix, or
(2) just a single 2.10.1 with everything late next week.
I can go either way and welcome suggestions. I'd start merging
older topics in the above list to 'maint' soonish, but not today.
Thanks.
^ permalink raw reply
* What's cooking in git.git (Sep 2016, #08; Tue, 27)
From: Junio C Hamano @ 2016-09-27 23:23 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
--------------------------------------------------
[Graduated to "master"]
* mh/diff-indent-heuristic (2016-09-19) 8 commits
(merged to 'next' on 2016-09-22 at e71d742)
+ blame: honor the diff heuristic options and config
+ parse-options: add parse_opt_unknown_cb()
+ diff: improve positioning of add/delete blocks in diffs
+ xdl_change_compact(): introduce the concept of a change group
+ recs_match(): take two xrecord_t pointers as arguments
+ is_blank_line(): take a single xrecord_t as argument
+ xdl_change_compact(): only use heuristic if group can't be matched
+ xdl_change_compact(): fix compaction heuristic to adjust ixo
Output from "git diff" can be made easier to read by selecting
which lines are common and which lines are added/deleted
intelligently when the lines before and after the changed section
are the same. A command line option is added to help with the
experiment to find a good heuristics.
* mh/diff-indent-heuristic (2016-09-27) 1 commit
- xdiff: rename "struct group" to "struct xdlgroup"
Clean-up for a recently graduated topic.
* rs/cocci (2016-09-15) 3 commits
(merged to 'next' on 2016-09-22 at aa54fa4)
+ use strbuf_addstr() for adding constant strings to a strbuf, part 2
+ add coccicheck make target
+ contrib/coccinelle: fix semantic patch for oid_to_hex_r()
Code cleanup.
* ep/doc-check-ref-format-example (2016-09-21) 1 commit
(merged to 'next' on 2016-09-22 at 6d0d79e)
+ git-check-ref-format.txt: fixup documentation
A shell script example in check-ref-format documentation has been
fixed.
* jk/clone-recursive-progress (2016-09-22) 1 commit
(merged to 'next' on 2016-09-22 at 8310c42)
+ clone: pass --progress decision to recursive submodules
"git clone --recurse-submodules" lost the progress eye-candy in
recent update, which has been corrected.
* jk/doc-cvs-update (2016-09-22) 3 commits
(merged to 'next' on 2016-09-22 at c0f949f)
+ docs/cvs-migration: mention cvsimport caveats
+ docs/cvs-migration: update link to cvsps homepage
+ docs/cvsimport: prefer cvs-fast-export to parsecvs
Documentation around tools to import from CVS was fairly outdated.
* js/regexec-buf (2016-09-21) 3 commits
(merged to 'next' on 2016-09-22 at 2ee2477)
+ regex: use regexec_buf()
+ regex: add regexec_buf() that can work on a non NUL-terminated string
+ regex: -G<pattern> feeds a non NUL-terminated string to regexec() and fails
Some codepaths in "git diff" used regexec(3) on a buffer that was
mmap(2)ed, which may not have a terminating NUL, leading to a read
beyond the end of the mapped region. This was fixed by introducing
a regexec_buf() helper that takes a <ptr,len> pair with REG_STARTEND
extension.
* jt/format-patch-rfc (2016-09-21) 1 commit
(merged to 'next' on 2016-09-22 at 3b39442)
+ format-patch: add "--rfc" for the common case of [RFC PATCH]
In some projects, it is common to use "[RFC PATCH]" as the subject
prefix for a patch meant for discussion rather than application. A
new option "--rfc" was a short-hand for "--subject-prefix=RFC PATCH"
to help the participants of such projects.
* ls/travis-homebrew-path-fix (2016-09-22) 1 commit
(merged to 'next' on 2016-09-22 at 310e620)
+ travis-ci: ask homebrew for its path instead of hardcoding it
The procedure to build Git on Mac OS X for Travis CI hardcoded the
internal directory structure we assumed HomeBrew uses, which was a
no-no. The procedure has been updated to ask HomeBrew things we
need to know to fix this.
* mm/config-color-ui-default-to-auto (2016-09-16) 1 commit
(merged to 'next' on 2016-09-22 at 4eac0cb)
+ Documentation/config: default for color.* is color.ui
Documentation for individual configuration variables to control use
of color (like `color.grep`) said that their default value is
'false', instead of saying their default is taken from `color.ui`.
When we updated the default value for color.ui from 'false' to
'auto' quite a while ago, all of them broke. This has been
corrected.
* nd/checkout-disambiguation (2016-09-21) 3 commits
(merged to 'next' on 2016-09-22 at ebfa365)
+ checkout: fix ambiguity check in subdir
+ checkout.txt: document a common case that ignores ambiguation rules
+ checkout: add some spaces between code and comment
"git checkout <word>" does not follow the usual disambiguation
rules when the <word> can be both a rev and a path, to allow
checking out a branch 'foo' in a project that happens to have a
file 'foo' in the working tree without having to disambiguate.
This was poorly documented and the check was incorrect when the
command was run from a subdirectory.
* rs/c-auto-resets-attributes (2016-09-19) 1 commit
(merged to 'next' on 2016-09-22 at 68f2e4a)
+ pretty: let %C(auto) reset all attributes
The pretty-format specifier "%C(auto)" used by the "log" family of
commands to enable coloring of the output is taught to also issue a
color-reset sequence to the output.
* rs/checkout-init-macro (2016-09-22) 1 commit
(merged to 'next' on 2016-09-22 at 6513755)
+ introduce CHECKOUT_INIT
Code cleanup.
* rt/rebase-i-broken-insn-advise (2016-09-07) 1 commit
(merged to 'next' on 2016-09-23 at 0d12484)
+ rebase -i: improve advice on bad instruction lines
When "git rebase -i" is given a broken instruction, it told the
user to fix it with "--edit-todo", but didn't say what the step
after that was (i.e. "--continue").
* tg/add-chmod+x-fix (2016-09-21) 6 commits
(merged to 'next' on 2016-09-22 at 6afdd21)
+ t3700-add: do not check working tree file mode without POSIXPERM
+ t3700-add: create subdirectory gently
+ add: modify already added files when --chmod is given
+ read-cache: introduce chmod_index_entry
+ update-index: add test for chmod flags
+ Merge branch 'ib/t3700-add-chmod-x-updates' into tg/add-chmod+x-fix
"git add --chmod=+x <pathspec>" added recently only toggled the
executable bit for paths that are either new or modified. This has
been corrected to flip the executable bit for all paths that match
the given pathspec.
* va/i18n-more (2016-09-21) 6 commits
(merged to 'next' on 2016-09-22 at bea26e8)
+ i18n: stash: mark messages for translation
+ i18n: notes-merge: mark die messages for translation
+ i18n: ident: mark hint for translation
+ i18n: i18n: diff: mark die messages for translation
+ i18n: connect: mark die messages for translation
+ i18n: commit: mark message for translation
Even more i18n.
--------------------------------------------------
[New Topics]
* mh/diff-indent-heuristic (2016-09-27) 1 commit
(merged to 'next' on 2016-09-27 at 3d6fb66)
+ xdiff: rename "struct group" to "struct xdlgroup"
Clean-up for a recently graduated topic.
Will merge to 'master'.
* rs/cocci (2016-09-27) 3 commits
- use strbuf_add_unique_abbrev() for adding short hashes, part 2
- use strbuf_addstr() instead of strbuf_addf() with "%s", part 2
- gitignore: ignore output files of coccicheck make target
Code clean-up with help from coccinelle tool continues.
Will merge to 'next'.
* jc/verify-loose-object-header (2016-09-26) 2 commits
(merged to 'next' on 2016-09-27 at 2947f95)
+ unpack_sha1_header(): detect malformed object header
+ streaming: make sure to notice corrupt object
Codepaths that read from an on-disk loose object were too loose in
validating what they are reading is a proper object file and
sometimes read past the data they read from the disk, which has
been corrected. H/t to Gustavo Grieco for reporting.
Will merge to 'master'.
* jk/ambiguous-short-object-names (2016-09-27) 11 commits
- get_short_sha1: make default disambiguation configurable
- get_short_sha1: list ambiguous objects on error
- for_each_abbrev: drop duplicate objects
- sha1_array: let callbacks interrupt iteration
- get_short_sha1: mark ambiguity error for translation
- get_short_sha1: NUL-terminate hex prefix
- get_short_sha1: refactor init of disambiguation code
- get_short_sha1: parse tags when looking for treeish
- get_sha1: propagate flags to child functions
- get_sha1: avoid repeating ourselves via ONLY_TO_DIE
- get_sha1: detect buggy calls with multiple disambiguators
When given an abbreviated object name that is not (or more
realistically, "no longer") unique, we gave a fatal error
"ambiguous argument". This error is now accompanied by hints that
lists the objects that begins with the given prefix. During the
course of development of this new feature, numerous minor bugs were
uncovered and corrected, the most notable one of which is that we
gave "short SHA1 xxxx is ambiguous." twice without good reason.
Will merge to 'next'.
* rs/copy-array (2016-09-25) 2 commits
(merged to 'next' on 2016-09-27 at c92e020)
+ use COPY_ARRAY
+ add COPY_ARRAY
Code cleanup.
Will merge to 'master'.
* rs/git-gui-use-modern-git-merge-syntax (2016-09-26) 2 commits
(merged to 'next' on 2016-09-27 at f55850d)
+ Merge branch 'rs/use-modern-git-merge-syntax' of git-gui into rs/git-gui-use-modern-git-merge-syntax
+ git-gui: stop using deprecated merge syntax
The original command line syntax for "git merge", which was "git
merge <msg> HEAD <parent>...", has been deprecated for quite some
time, and "git gui" was the last in-tree user of the syntax. This
is finally fixed, so that we can move forward with the deprecation.
Will merge to 'master'.
* va/git-gui-i18n (2016-09-26) 3 commits
(merged to 'next' on 2016-09-27 at ab0f66f)
+ Merge branch 'va/i18n' of ../git-gui into va/git-gui-i18n
+ git-gui: l10n: add Portuguese translation
+ git-gui i18n: mark strings for translation
"git gui" l10n to Portuguese.
Will merge to 'master'.
* va/i18n-perl-scripts (2016-09-25) 11 commits
- i18n: difftool: mark warnings for translation
- i18n: send-email: mark string with interpolation for translation
- i18n: send-email: mark warnings and errors for translation
- i18n: send-email: mark strings for translation
- i18n: add--interactive: mark edit_hunk_manually message for translation
- i18n: add--interactive: i18n of help_patch_cmd
- i18n: add--interactive: mark message for translation
- i18n: add--interactive: mark plural strings
- i18n: add--interactive: mark strings with interpolation for translation
- i18n: add--interactive: mark simple here documents for translation
- i18n: add--interactive: mark strings for translation
Porcelain scripts written in Perl are getting internationalized.
Waiting for a reroll.
cf. <1474913721.1035.9.camel@sapo.pt>
* vn/revision-shorthand-for-side-branch-log (2016-09-27) 1 commit
- revision: new rev^-n shorthand for rev^n..rev
"git log rev^..rev" is an often-used revision range specification
to show what was done on a side branch merged at rev, which has
gained a short-hand "rev^-1". In general "rev^-$n" is the same as
"^rev^$n rev", i.e. what was done while the history leading to nth
parent was looking the other way.
Will merge to 'next'.
* dt/mailinfo (2016-09-26) 1 commit
(merged to 'next' on 2016-09-27 at 59e95db)
+ add David Turner's Two Sigma address
Will merge to 'master'.
* dt/tree-fsck (2016-09-27) 2 commits
- fsck: handle bad trees like other errors
- tree-walk: be more specific about corrupt tree errors
The codepath in "git fsck" to detect malformed tree objects has
been updated not to die but keep going after detecting them.
Will merge to 'next'.
* jc/latin-1 (2016-09-26) 2 commits
- utf8: accept "latin-1" as ISO-8859-1
- utf8: refactor code to decide fallback encoding
Some platforms no longer understand "latin-1" that is still seen in
the wild in e-mail headers; replace them with "iso-8859-1" that is
more widely known when conversion fails from/to it.
Will merge to 'next'.
* jc/worktree-config (2016-09-27) 1 commit
- worktree: honor configuration variables
"git worktree", even though it used the default_abbrev setting that
ought to be affected by core.abbrev configuration variable, ignored
the variable setting. The command has been taught to read the
default set of configuration variables to correct this.
Will merge to 'next'.
* mg/gpg-richer-status (2016-09-27) 1 commit
- gpg-interface: use more status letters
The GPG verification status shown in "%G?" pretty format specifier
was not rich enough to differentiate a signature made by an expired
key, a signature made by a revoked key, etc. New output letters
have been assigned to express them.
Will merge to 'next'.
* pb/rev-list-reverse-with-count (2016-09-27) 1 commit
- rev-list-options: clarify the usage of --reverse
Doc update.
Will merge to 'next'.
--------------------------------------------------
[Stalled]
* 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/attr (2016-05-25) 18 commits
- attr: support quoting pathname patterns in C style
- attr: expose validity check for attribute names
- attr: add counted string version of git_attr()
- 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
(this branch is used by jc/attr-more, sb/pathspec-label and sb/submodule-default-paths.)
The attributes API has been updated so that it can later be
optimized using the knowledge of which attributes are queried.
I wanted to polish this topic further to make the attribute
subsystem thread-ready, but because other topics depend on this
topic and they do not (yet) need it to be thread-ready.
As the authors of topics that depend on this seem not in a hurry,
let's discard this and dependent topics and restart them some other
day.
Will discard.
* jc/attr-more (2016-06-09) 8 commits
- 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
- fixup! d5ad6c13
- attr.c: pass struct git_attr_check down the callchain
- attr.c: add push_stack() helper
(this branch uses jc/attr; is tangled with sb/pathspec-label and sb/submodule-default-paths.)
The beginning of long and tortuous journey to clean-up attribute
subsystem implementation.
Needs to be redone.
Will discard.
* sb/submodule-default-paths (2016-06-20) 5 commits
- completion: clone can recurse into submodules
- clone: add --init-submodule=<pathspec> switch
- submodule update: add `--init-default-path` switch
- Merge branch 'sb/pathspec-label' into sb/submodule-default-paths
- Merge branch 'jc/attr' into sb/submodule-default-paths
(this branch uses jc/attr and sb/pathspec-label; is tangled with jc/attr-more.)
Allow specifying the set of submodules the user is interested in on
the command line of "git clone" that clones the superproject.
Will discard.
* sb/pathspec-label (2016-06-03) 6 commits
- pathspec: disable preload-index when attribute pathspec magic is in use
- 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
(this branch is used by sb/submodule-default-paths; uses jc/attr; is tangled with jc/attr-more.)
The pathspec mechanism learned ":(attr:X)$pattern" pathspec magic
to limit paths that match $pattern further by attribute settings.
The preload-index mechanism is disabled when the new pathspec magic
is in use (at least for now), because the attribute subsystem is
not thread-ready.
Will discard.
* mh/connect (2016-06-06) 10 commits
- connect: [host:port] is legacy for ssh
- connect: move ssh command line preparation to a separate function
- connect: actively reject git:// urls with a user part
- connect: change the --diag-url output to separate user and host
- connect: make parse_connect_url() return the user part of the url as a separate value
- connect: group CONNECT_DIAG_URL handling code
- connect: make parse_connect_url() return separated host and port
- connect: re-derive a host:port string from the separate host and port variables
- connect: call get_host_and_port() earlier
- connect: document why we sometimes call get_port after get_host_and_port
Rewrite Git-URL parsing routine (hopefully) without changing any
behaviour.
It has been two months without any support. We may want to discard
this.
* pb/bisect (2016-08-23) 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 `--check-and-set-terms` 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
. 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
GSoC "bisect" topic.
I'd prefer to see early part solidified so that reviews can focus
on the later part that is still in flux. We are almost there but
not quite yet.
* kn/ref-filter-branch-list (2016-05-17) 17 commits
- 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 `:dir` and `:base` options for ref printing atoms
- ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
- ref-filter: introduce symref_atom_parser() and 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.
Rerolled.
Needs review.
* sb/bisect (2016-04-15) 22 commits
. SQUASH???
. bisect: get back halfway shortcut
. bisect: compute best bisection in compute_relevant_weights()
. bisect: use a bottom-up traversal to find relevant weights
. bisect: prepare for different algorithms based on find_all
. bisect: rename count_distance() to compute_weight()
. bisect: make total number of commits global
. bisect: introduce distance_direction()
. bisect: extract get_distance() function from code duplication
. bisect: use commit instead of commit list as arguments when appropriate
. bisect: replace clear_distance() by unique markers
. bisect: use struct node_data array instead of int array
. bisect: get rid of recursion in count_distance()
. bisect: make algorithm behavior independent of DEBUG_BISECT
. bisect: make bisect compile if DEBUG_BISECT is set
. bisect: plug the biggest memory leak
. bisect: add test for the bisect algorithm
. t6030: generalize test to not rely on current implementation
. t: use test_cmp_rev() where appropriate
. t/test-lib-functions.sh: generalize test_cmp_rev
. bisect: allow 'bisect run' if no good commit is known
. bisect: write about `bisect next` in documentation
The internal algorithm used in "git bisect" to find the next commit
to check has been optimized greatly.
Was expecting a reroll, but now pb/bisect topic starts removinging
more and more parts from git-bisect.sh, this needs to see a fresh
reroll.
Will discard.
cf. <1460294354-7031-1-git-send-email-s-beyer@gmx.net>
* sg/completion-updates (2016-02-28) 21 commits
. completion: cache the path to the repository
. completion: extract repository discovery from __gitdir()
. completion: don't guard git executions with __gitdir()
. completion: consolidate silencing errors from git commands
. completion: don't use __gitdir() for git commands
. completion: respect 'git -C <path>'
. completion: fix completion after 'git -C <path>'
. completion: don't offer commands when 'git --opt' needs an argument
. rev-parse: add '--absolute-git-dir' option
. completion: list short refs from a remote given as a URL
. completion: don't list 'HEAD' when trying refs completion outside of a repo
. completion: list refs from remote when remote's name matches a directory
. completion: respect 'git --git-dir=<path>' when listing remote refs
. completion: fix most spots not respecting 'git --git-dir=<path>'
. completion: ensure that the repository path given on the command line exists
. completion tests: add tests for the __git_refs() helper function
. completion tests: check __gitdir()'s output in the error cases
. completion tests: consolidate getting path of current working directory
. completion tests: make the $cur variable local to the test helper functions
. completion tests: don't add test cruft to the test repository
. completion: improve __git_refs()'s in-code documentation
Has been waiting for a reroll for too long.
cf. <1456754714-25237-1-git-send-email-szeder@ira.uka.de>
Will discard.
* ec/annotate-deleted (2015-11-20) 1 commit
- annotate: skip checking working tree if a revision is provided
Usability fix for annotate-specific "<file> <rev>" syntax with deleted
files.
Has been waiting for a review for too long without seeing anything.
Will discard.
* dk/gc-more-wo-pack (2016-01-13) 4 commits
- gc: clean garbage .bitmap files from pack dir
- t5304: ensure non-garbage files are not deleted
- t5304: test .bitmap garbage files
- prepare_packed_git(): find more garbage
Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
.bitmap and .keep files.
Has been waiting for a reroll for too long.
cf. <xmqq60ypbeng.fsf@gitster.mtv.corp.google.com>
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]
* jk/verify-packfile-gently (2016-09-22) 1 commit
(merged to 'next' on 2016-09-26 at f5abba5)
+ verify_packfile: check pack validity before accessing data
A low-level function verify_packfile() was meant to show errors
that were detected without dying itself, but under some conditions
it didn't and died instead, which has been fixed.
Will merge to 'master'.
* jt/fetch-pack-in-vain-count-with-stateless (2016-09-23) 1 commit
(merged to 'next' on 2016-09-26 at 9645629)
+ fetch-pack: do not reset in_vain on non-novel acks
When "git fetch" tries to find where the history of the repository
it runs inhas diverged from what the other side has, it has a
mechanism to avoid digging too deep into irrelevant side branches.
This however did not work well over the "smart-http" transport due
to a design bug, which has been fixed.
Will merge to 'master'.
* ik/gitweb-force-highlight (2016-09-25) 2 commits
(merged to 'next' on 2016-09-27 at cbb8391)
+ gitweb: use highlight's shebang detection
+ gitweb: remove unused guess_file_syntax() parameter
"gitweb" can spawn "highlight" to show blob contents with
(programming) language-specific syntax highlighting, but only
when the language is known. "highlight" can however be told
to make the guess itself by giving it "--force" option, which
has been enabled.
Will merge to 'master'.
* jk/ident-ai-canonname-could-be-null (2016-09-23) 1 commit
(merged to 'next' on 2016-09-26 at 0eefb29)
+ ident: handle NULL ai_canonname
In the codepath that comes up with the hostname to be used in an
e-mail when the user didn't tell us, we looked at ai_canonname
field in struct addrinfo without making sure it is not NULL first.
Will merge to 'master'.
* jc/blame-reverse (2016-06-14) 2 commits
(merged to 'next' on 2016-09-22 at d1a8e9c)
+ blame: dwim "blame --reverse OLD" as "blame --reverse OLD.."
+ blame: improve diagnosis for "--reverse NEW"
It is a common mistake to say "git blame --reverse OLD path",
expecting that the command line is dwimmed as if asking how lines
in path in an old revision OLD have survived up to the current
commit.
Will hold to see if it is broken.
* nd/init-core-worktree-in-multi-worktree-world (2016-09-25) 5 commits
(merged to 'next' on 2016-09-27 at 619f7f3)
+ init: kill git_link variable
+ init: do not set unnecessary core.worktree
+ init: kill set_git_dir_init()
+ init: call set_git_dir_init() from within init_db()
+ init: correct re-initialization from a linked worktree
"git init" tried to record core.worktree in the repository's
'config' file when GIT_WORK_TREE environment variable was set and
it was different from where GIT_DIR appears as ".git" at its top,
but the logic was faulty when .git is a "gitdir:" file that points
at the real place, causing trouble in working trees that are
managed by "git worktree". This has been corrected.
Will merge to 'master'.
* jt/mailinfo-fold-in-body-headers (2016-09-21) 3 commits
(merged to 'next' on 2016-09-26 at 4235eb6)
+ mailinfo: handle in-body header continuations
+ mailinfo: make is_scissors_line take plain char *
+ mailinfo: separate in-body header processing
When "git format-patch --stdout" output is placed as an in-body
header and it uses the RFC2822 header folding, "git am" failed to
put the header line back into a single logical line. The
underlying "git mailinfo" was taught to handle this properly.
Will merge to 'master'.
* kd/mailinfo-quoted-string (2016-09-26) 3 commits
- mailinfo: unescape quoted-pair in header fields
- SQUASH???
- t5100-mailinfo: replace common path prefix with variable
An e-mail author named that spelled a backslash-quoted double quote
in the human readable part "My \"double quoted\" name" was not
unquoted correctly.
Waiting for a reroll.
cf. <20160926194455.GB19089@ikke.info>
* js/libify-require-clean-work-tree (2016-09-12) 5 commits
- wt-status: teach has_{unstaged,uncommitted}_changes() about submodules
- Export also the has_un{staged,committed}_changed() functions
- Make the require_clean_work_tree() function truly reusable
- pull: make code more similar to the shell script again
- pull: drop confusing prefix parameter of die_on_unclean_work_tree()
The require_clean_work_tree() helper was recreated in C when "git
pull" was rewritten from shell; the helper is now made available to
other callers in preparation for upcoming "rebase -i" work.
Waiting for comments.
Modulo a few minor nits, this looked almost ready.
cf. <xmqqtwdl2bhm.fsf@gitster.mtv.corp.google.com>
cf. <xmqqpoo92bdr.fsf@gitster.mtv.corp.google.com>
* bw/ls-files-recurse-submodules (2016-09-21) 2 commits
- ls-files: add pathspec matching for submodules
- ls-files: optionally recurse into submodules
"git ls-files" learned "--recurse-submodules" option that can be
used to get a listing of tracked files across submodules (i.e. this
only works with "--cached" option, not for listing untracked or
ignored files). This would be a useful tool to sit on the upstream
side of a pipe that is read with xargs to work on all working tree
files from the top-level superproject.
Waiting for the discussion to conclude.
* ls/filter-process (2016-09-23) 11 commits
- convert: add filter.<driver>.process option
- convert: make apply_filter() adhere to standard Git error handling
- convert: modernize tests
- convert: quote filter names in error messages
- pkt-line: add functions to read/write flush terminated packet streams
- pkt-line: add packet_write_gently()
- pkt-line: add packet_flush_gently()
- pkt-line: add packet_write_fmt_gently()
- run-command: move check_pipe() from write_or_die to run_command
- pkt-line: extract set_packet_header()
- pkt-line: rename packet_write() to packet_write_fmt()
The smudge/clean filter API expect an external process is spawned
to filter the contents for each path that has a filter defined. A
new type of "process" filter API has been added to allow the first
request to run the filter for a path to spawn a single process, and
all filtering need is served by this single process for multiple
paths, reducing the process creation overhead.
* hv/submodule-not-yet-pushed-fix (2016-09-14) 2 commits
- serialize collection of refs that contain submodule changes
- serialize collection of changed submodules
The code in "git push" to compute if any commit being pushed in the
superproject binds a commit in a submodule that hasn't been pushed
out was overly inefficient, making it unusable even for a small
project that does not have any submodule but have a reasonable
number of refs.
The last two in the original series seem to break a few tests when
queued to 'pu', and dropped for now.
Waiting for a reroll.
* sg/fix-versioncmp-with-common-suffix (2016-09-08) 5 commits
- versioncmp: cope with common leading parts in versionsort.prereleaseSuffix
- 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).
Waiting for a reroll.
cf. <20160908223727.Horde.jVOOJ278ssZ3qkyjkmyqZD-@webmail.informatik.kit.edu>
* cp/completion-negative-refs (2016-08-24) 1 commit
(merged to 'next' on 2016-09-22 at abd1585)
+ completion: support excluding refs
The command-line completion script (in contrib/) learned to
complete "git cmd ^mas<HT>" to complete the negative end of
reference to "git cmd ^master".
Will hold to see if it is broken.
* sb/push-make-submodule-check-the-default (2016-08-24) 1 commit
- push: change submodule default to check
Turn the default of "push.recurseSubmodules" to "check".
Will hold to wait for hv/submodule-not-yet-pushed-fix
This reveals that the "check" mode is too inefficient to use in
real projects, even in ones as small as git itself.
cf. <xmqqh9aaot49.fsf@gitster.mtv.corp.google.com>
* ak/curl-imap-send-explicit-scheme (2016-08-17) 1 commit
(merged to 'next' on 2016-09-22 at 4449584)
+ imap-send: Tell cURL to use imap:// or imaps://
When we started cURL to talk to imap server when a new enough
version of cURL library is available, we forgot to explicitly add
imap(s):// before the destination. To some folks, that didn't work
and the library tried to make HTTP(s) requests instead.
Will hold to see if it is broken.
* jk/pack-objects-optim-mru (2016-08-11) 4 commits
(merged to 'next' on 2016-09-21 at 97b919b)
+ pack-objects: use mru list when iterating over packs
+ pack-objects: break delta cycles before delta-search phase
+ sha1_file: make packed_object_info public
+ provide an initializer for "struct object_info"
Originally merged to 'next' on 2016-08-11
"git pack-objects" in a repository with many packfiles used to
spend a lot of time looking for/at objects in them; the accesses to
the packfiles are now optimized by checking the most-recently-used
packfile first.
Will hold to see if people scream.
* dp/autoconf-curl-ssl (2016-06-28) 1 commit
(merged to 'next' on 2016-09-22 at 9c5aeec)
+ ./configure.ac: detect SSL in libcurl using curl-config
The ./configure script generated from configure.ac was taught how
to detect support of SSL by libcurl better.
Will hold to see if it is broken.
* jc/pull-rebase-ff (2016-07-28) 1 commit
- pull: fast-forward "pull --rebase=true"
"git pull --rebase", when there is no new commits on our side since
we forked from the upstream, should be able to fast-forward without
invoking "git rebase", but it didn't.
Needs a real log message and a few tests.
* ex/deprecate-empty-pathspec-as-match-all (2016-06-22) 1 commit
(merged to 'next' on 2016-09-21 at e19148e)
+ pathspec: warn on empty strings as pathspec
Originally merged to 'next' on 2016-07-13
An empty string used as a pathspec element has always meant
'everything matches', but it is too easy to write a script that
finds a path to remove in $path and run 'git rm "$paht"', which
ends up removing everything. Start warning about this use of an
empty string used for 'everything matches' and ask users to use a
more explicit '.' for that instead.
The hope is that existing users will not mind this change, and
eventually the warning can be turned into a hard error, upgrading
the deprecation into removal of this (mis)feature.
Will hold to see if people scream.
* nd/shallow-deepen (2016-06-13) 27 commits
(merged to 'next' on 2016-09-22 at f0cf3e3)
+ fetch, upload-pack: --deepen=N extends shallow boundary by N commits
+ upload-pack: add get_reachable_list()
+ upload-pack: split check_unreachable() in two, prep for get_reachable_list()
+ t5500, t5539: tests for shallow depth excluding a ref
+ clone: define shallow clone boundary with --shallow-exclude
+ fetch: define shallow boundary with --shallow-exclude
+ upload-pack: support define shallow boundary by excluding revisions
+ refs: add expand_ref()
+ t5500, t5539: tests for shallow depth since a specific date
+ clone: define shallow clone boundary based on time with --shallow-since
+ fetch: define shallow boundary with --shallow-since
+ upload-pack: add deepen-since to cut shallow repos based on time
+ shallow.c: implement a generic shallow boundary finder based on rev-list
+ fetch-pack: use a separate flag for fetch in deepening mode
+ fetch-pack.c: mark strings for translating
+ fetch-pack: use a common function for verbose printing
+ fetch-pack: use skip_prefix() instead of starts_with()
+ upload-pack: move rev-list code out of check_non_tip()
+ upload-pack: make check_non_tip() clean things up on error
+ upload-pack: tighten number parsing at "deepen" lines
+ upload-pack: use skip_prefix() instead of starts_with()
+ upload-pack: move "unshallow" sending code out of deepen()
+ upload-pack: remove unused variable "backup"
+ upload-pack: move "shallow" sending code out of deepen()
+ upload-pack: move shallow deepen code out of receive_needs()
+ transport-helper.c: refactor set_helper_option()
+ remote-curl.c: convert fetch_git() to use argv_array
The existing "git fetch --depth=<n>" option was hard to use
correctly when making the history of an existing shallow clone
deeper. A new option, "--deepen=<n>", has been added to make this
easier to use. "git clone" also learned "--shallow-since=<date>"
and "--shallow-exclude=<tag>" options to make it easier to specify
"I am interested only in the recent N months worth of history" and
"Give me only the history since that version".
Will hold to see if it is broken.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
- merge: drop 'git merge <message> HEAD <commit>' syntax
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.
It has been reported that git-gui still uses the deprecated syntax,
which needs to be fixed before this final step can proceed.
cf. <5671DB28.8020901@kdbg.org>
Will hold to wait for rs/git-gui-use-modern-git-merge-syntax
--------------------------------------------------
[Discarded]
* jn/fix-connect-unexpected-hangup-diag (2016-09-08) 1 commit
. connect: tighten check for unexpected early hang up
Now part of jt/accept-capability-advertisement-when-fetching-from-void
topic.
^ permalink raw reply
* Re: [PATCH 3/3] docs/cvs-migration: mention cvsimport caveats
From: Eric S. Raymond @ 2016-09-28 0:11 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20160923035601.f5czdieouzvm3ptz@sigill.intra.peff.net>
Jeff King <peff@peff.net>:
> I am not qualified to write on the current state of
> the art in CVS importing.
I *am* qualified; cvs-fast-export has had a lot of work put into it by
myself and others over the last five years. Nobody else is really
working this problem anymore, not much else than cvs2git is even left
standing at this point. Most other attempts on the problem have
stalled or flamed out, and were never very robust in dealing with
repository malformations to begin with.
cvs2git can probably still almost match cvs-fast-export in ability to handle
pathological cases, but is painfully slow by comparison. (Part of that is
implementation in Python vs. C.)
cvs-fast-export has been successfully performance-tuned for very large
repositories, such as the entirety of NetBSD, and is orders of
magnitude faster than it used to be. (I parallelized the parsing
of RCS masters with a re-entrant Bison instance running per thread;
this makes a huge difference on large repositories, for which that
stage dominates running time.) Its ability to recover sense from
repository malformations was already pretty good five years ago
and is probably unmatched now. It does .cvsignore conversion.
cvs-fast-export also now has a really good test suite collecting all
kinds of weird CVS deformations from the field, and a wrapper that can
both do a conversion and check for correctness at every tag as well as
the tip revision.
By contrast, the wrapper/cvsps combination git ships continues to be
disgracefully bad and should be scrapped - remember that I maintained
cvsps for a while and tried to EOL it because its branch-resolution
algorithms are unsound. I have a replacement wrapper ready any time
the git maintainer decides to stop shipping broken, dangerous code.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: [PATCH v2] rev-list-options: clarify the usage of --reverse
From: Pranit Bauva @ 2016-09-28 1:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Philip Oakley, Git List
In-Reply-To: <xmqqoa39kn57.fsf@gitster.mtv.corp.google.com>
Hey Junio,
On Wed, Sep 28, 2016 at 3:42 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> micro-nit: 'first' and 'last' can be tricky to distinguish for lists...
>
> Let's do this then.
>
> -- >8 --
> From: Pranit Bauva <pranit.bauva@gmail.com>
> Date: Tue, 27 Sep 2016 20:44:09 +0000
> Subject: [PATCH] rev-list-options: clarify the usage of --reverse
>
> Users often wonder if the oldest or the newest n commits are shown
> by `log -n --reverse`. Clarify that --reverse kicks in only after
> deciding which commits are to be shown to unconfuse them.
>
> Reported-by: Ruediger Meier <sweet_f_a@gmx.de>
> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
Looks good! Thanks!
> Documentation/rev-list-options.txt | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 4f009d4..f8946d4 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -638,8 +638,9 @@ avoid showing the commits from two parallel development track mixed
> together.
>
> --reverse::
> - Output the commits in reverse order.
> - Cannot be combined with `--walk-reflogs`.
> + Output the commits chosen to be shown (see Commit Limiting
> + section above) in reverse order. Cannot be combined with
> + `--walk-reflogs`.
>
> Object Traversal
> ~~~~~~~~~~~~~~~~
> --
> 2.10.0-561-g98a6b79
>
^ permalink raw reply
* Repeatable Extraction
From: chris king @ 2016-09-28 3:14 UTC (permalink / raw)
To: git
Hello, first off thanks for such a wonderful tool! I have a general
question and I hope this is an appropriate spot to ask it.
Is there a way automate extraction that will repeatably generate the
same files? Currently, each time I extract git portable many of the
binaries change slightly. For example, if I extract twice using
PortableGit-2.10.0-32-bit.7z.exe -y -gm2
then Beyond Compare tells me that many of the files in usr\bin have
changed at offset 0x88 and 0x89. Why is that?
Warm regards,
Chris
^ permalink raw reply
* Re: [PATCH 01/11] Resumable clone: create service git-prime-clone
From: Kevin Wern @ 2016-09-28 4:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kevin Wern, git
In-Reply-To: <xmqqzin7in2c.fsf@gitster.mtv.corp.google.com>
On Fri, Sep 16, 2016 at 01:53:15PM -0700, Junio C Hamano wrote:
> Kevin Wern <kevin.m.wern@gmail.com> writes:
>
> > Create git-prime-clone, a program to be executed on the server that
> > returns the location and type of static resource to download before
> > performing the rest of a clone.
> >
> > Additionally, as this executable's location will be configurable (see:
> > upload-pack and receive-pack), add the program to
> > BINDIR_PROGRAMS_NEED_X, in addition to the usual builtin places. Add
> > git-prime-clone executable to gitignore, as well
> >
> > Signed-off-by: Kevin Wern <kevin.m.wern@gmail.com>
> > ---
>
> I wonder if we even need a separate service like this.
>
> Wouldn't a new protocol capability that is advertised from
> upload-pack sufficient to tell the "git clone" that it can
> and should consider priming from this static resource?
The short answer is yes, it could be done that way. Both methods--extending
upload-pack and creating a new service--were suggested in different
discussions.
However, my thought was to implement the separate service because:
- It is much easier for an admin trying the feature to sanity check the
output of an executable, compared to passing messages to upload-pack.
- In the other scenario, upload-pack might get too expansive in size
and scope--not only codewise, but in terms of config namespace if
"uploadpack" concerns too many things that are only tangentially
related (the properties of the primer resource).
- The transport_prime_clone API can be called independent of other
transport API functions, which might prove useful when revisiting or
refactoring code.
- You favored the creation of a service in our original discussion [1].
I'm not sure if your reasoning was similar to mine.
It definitely was a tight decision--for me, ultimately weighing the value added
in usability (point 1) against the need for a "failsafe" implementation. All
the other points are more speculative, IMO, but the first was strong enough for
me.
What do you think?
[1] http://www.spinics.net/lists/git/msg269992.html
> Two minor comments:
>
> - For whom are you going to localize these strings? This program
> is running on the server side and we do not know the locale
> preferred by the end-user who is sitting on the other end of the
> connection, no?
>
> - Turn "}\n\s+else " into "} else ", please.
These are fair points. Changing for the revised version.
- Kevin
^ permalink raw reply
* Re: [PATCH 03/11] pkt-line: create gentle packet_read_line functions
From: Kevin Wern @ 2016-09-28 4:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kevin Wern, git
In-Reply-To: <xmqqoa3nij5z.fsf@gitster.mtv.corp.google.com>
On Fri, Sep 16, 2016 at 03:17:28PM -0700, Junio C Hamano wrote:
> Kevin Wern <kevin.m.wern@gmail.com> writes:
>
> > /* And complain if we didn't get enough bytes to satisfy the read. */
> > if (ret < size) {
> > - if (options & PACKET_READ_GENTLE_ON_EOF)
> > + if (options & (PACKET_READ_GENTLE_ON_EOF | PACKET_READ_GENTLE_ALL))
> > return -1;
>
> The name _ALL suggested to me that there may be multiple "under this
> condition, be gentle", "under that condition, be gentle", and _ALL
> is used as a catch-all "under any condition, be gentle". If you
> defined _ALL symbol to have all GENTLE bits on, this line could have
> become
>
> if (options & PACKET_READ_GENTLE_ALL)
>
> > @@ -205,15 +209,23 @@ int packet_read(int fd, char **src_buf, size_t *src_len,
> > if (ret < 0)
> > return ret;
> > len = packet_length(linelen);
> > - if (len < 0)
> > + if (len < 0) {
> > + if (options & PACKET_READ_GENTLE_ALL)
> > + return -1;
>
> On the other hand, however, you do want to die here when only
> GENTLE_ON_EOF is set.
>
> Taking the above two observations together, I'd have to say that
> _ALL is probably a misnomer. I agree with a need for a flag with
> the behaviour you defined in this patch, though.
OK, my thought is either:
- Come up with a name for a flag, or flags, for the other cases (to
check in the function, i.e. PACKET_READ_GENTLE_INVALID), and still
pass in PACKET_READ_GENTLE_ALL, which is all those bits on plus
*_GENTLE_ON_EOF.
- Come up with a better name for this single flag, like
PACKET_READ_DONT_DIE ... only better.
What would you suggest here?
^ permalink raw reply
* Re: [PATCH 02/11] Resumable clone: add prime-clone endpoints
From: Kevin Wern @ 2016-09-28 4:43 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Kevin Wern, Git Mailing List
In-Reply-To: <CACsJy8ARu_PkduP-McnE26OFEgr_YAkLtmfaxwYX7i9m47mJmg@mail.gmail.com>
On Mon, Sep 19, 2016 at 08:15:00PM +0700, Duy Nguyen wrote:
> We also have an exception for select_getanyfile() below. I think it's
> time we add a function callback in struct rpc_service to run each
> service the way they want. Then prime-clone won't need an exception
> (neither does select_anyfile, mostly)
> --
> Duy
Great idea! I'll definitely do that if we decide to go with the service I
implemented.
^ permalink raw reply
* Re: [PATCH 11/11] Resumable clone: implement primer logic in git-clone
From: Kevin Wern @ 2016-09-28 4:44 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Kevin Wern, Git Mailing List
In-Reply-To: <CACsJy8B1bbKBhg1ke4u6PV3k4FWz-bhBPyN2X=mV2Z2=8Mhy=A@mail.gmail.com>
On Mon, Sep 19, 2016 at 09:04:40PM +0700, Duy Nguyen wrote:
> On Fri, Sep 16, 2016 at 7:12 AM, Kevin Wern <kevin.m.wern@gmail.com> wrote:
> > builtin/clone.c | 590 +++++++++++++++++++++++++++++++++++++-------
>
> Argh.. this is too big for my brain at this hour. It might be easier
> to follow if you separate out some code move (I think I've seen some,
> not sure). I'll try to have another look when I find time. But it's
> great to hear from you again, the pleasant surprise in my inbox today,
> as I thought we lost you ;-) There's hope for resumable clone maybe
> before 2018 again.
Sorry, I didn't mean to hurt anyone haha. I probably should have broken it down
into the bare process (priming from static resource, with no options), and the
resume option.
Thanks so much for your feedback! :)
^ permalink raw reply
* Re: [PATCH 10/11] run command: add RUN_COMMAND_NO_STDOUT
From: Kevin Wern @ 2016-09-28 4:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kevin Wern, git
In-Reply-To: <xmqq37kzigvf.fsf@gitster.mtv.corp.google.com>
On Fri, Sep 16, 2016 at 04:07:00PM -0700, Junio C Hamano wrote:
> Kevin Wern <kevin.m.wern@gmail.com> writes:
>
> > Add option RUN_COMMAND_NO_STDOUT, which sets no_stdout on a child
> > process.
> >
> > This will be used by git clone when calling index-pack on a downloaded
> > packfile.
>
> If it is just one caller, would't it make more sense for that caller
> set no_stdout explicitly itself?
I based the calling code in do_index_pack on dissociate_from_references, which
uses run_command_v_opt, so it never occured to me to do that. I thought it was
just good, uniform style and encapsulation. Like how transport's methods and
internals aren't really intended to be changed or accessed--unless it's through
the APIs we create.
However, I don't feel very strongly about this, so I'm okay with this change.
^ permalink raw reply
* Re: thoughts on error passing, was Re: [PATCH 2/2] fsck: handle bad trees like other errors
From: Michael Haggerty @ 2016-09-28 5:01 UTC (permalink / raw)
To: Jeff King, David Turner; +Cc: git, David Turner
In-Reply-To: <20160927191955.mympqgylrxhkp24n@sigill.intra.peff.net>
On 09/27/2016 09:19 PM, Jeff King wrote:
> [...]
> I'm going to ramble for a minute, and I don't think it's worth exploring
> for this patch series in particular, so feel free to ignore me.
>
> I think this error concept could be extended fairly elegantly with
> something like:
>
> typedef void (*err_fn)(void *, const char *fmt, va_list ap)
> struct error_context {
> err_fn fn;
> void *data;
> };
>
> int report_error(struct error_context *err, const char *fmt, ...)
> {
> if (err->fn) {
> va_list ap;
> va_start(ap, fmt);
> err->fn(err->data, fmt, ap);
> va_end(ap);
> }
> return -1;
> }
I like this idea. It's nicely flexible (more so than the `struct strbuf
*err` that is currently used for reference transactions) without being
cumbersome.
> Then low-level functions just take a context and do:
>
> return report_error(&err, "some error: %s", foo);
>
> And then the callers would pick one of a few generic error contexts:
>
> - passing NULL silences the errors
>
> - a global for chaining to error, like:
>
> struct error_context print_errors = {
> error, /* actually a wrapper to handle va_list and NULL data */
> NULL
> };
There could also be a global for chaining to `warn()` or `die()`.
> [...]
Michael
^ permalink raw reply
* Re: [PATCH 11/11] Resumable clone: implement primer logic in git-clone
From: Kevin Wern @ 2016-09-28 5:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kevin Wern, git
In-Reply-To: <xmqqy42rh156.fsf@gitster.mtv.corp.google.com>
On Fri, Sep 16, 2016 at 04:32:05PM -0700, Junio C Hamano wrote:
> > -----------
> > @@ -172,6 +173,12 @@ objects from the source repository into a pack in the cloned repository.
> > via ssh, this specifies a non-default path for the command
> > run on the other end.
> >
> > +--prime-clone <prime-clone>::
> > +-p <prime-clone>::
>
> Not many other options have single letter shorthand. Is it expected
> that it is worth to let this option squat on a short-and-sweet "-p",
> perhaps because it is so frequently used?
I based my decision more on precedent than value--the "--upload-pack" option
had the shorthand "-u".
> > @@ -40,17 +42,20 @@ static const char * const builtin_clone_usage[] = {
> >
> > static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
> > static int option_local = -1, option_no_hardlinks, option_shared, option_recursive;
> > +static int option_resume;
> > static char *option_template, *option_depth;
> > -static char *option_origin = NULL;
> > +static const char *option_origin = NULL;
>
> Is this change related to anything you are doing here?
>
> If you are fixing things while at it, please don't ;-) If you really
> want to, please also remove " = NULL", from this line and also from
> the next line. Also do not add " = NULL" at the end of alt_res.
>
Yes, it was because remote_config uses all const char* strings, and .name gets
assigned to option_origin when it pulls the previous info from config. I looked
to see if option_origin's underlying string is ever modified in a way that
makes it ineligible to be const, and it wasn't.
So, I should remove the NULLs here? And then initalize alt_res to NULL in
cmd_clone?
> > static char *option_branch = NULL;
> > ...
> > +static const struct alt_resource *alt_res = NULL;
>
> > +static char *get_filename(const char *dir)
> > +{
> > + char *dir_copy = xstrdup(dir);
> > + strip_trailing_slashes(dir_copy);
> > + char *filename, *final = NULL;
> > +
> > + filename = find_last_dir_sep(dir);
> > +
> > + if (filename && *(++filename))
> > + final = xstrdup(filename);
> > +
> > + free(dir_copy);
> > + return final;
> > +}
>
> Hmph, don't we have our own basename(3) lookalike that knows about
> dir-sep already?
Whoops, did not catch that. Thanks!
> > @@ -562,7 +614,7 @@ static void write_remote_refs(const struct ref *local_refs)
> > die("%s", err.buf);
> >
> > for (r = local_refs; r; r = r->next) {
> > - if (!r->peer_ref)
> > + if (!r->peer_ref || ref_exists(r->peer_ref->name))
> > continue;
> > if (ref_transaction_create(t, r->peer_ref->name, r->old_oid.hash,
> > 0, NULL, &err))
>
> What is this change about?
Because resumable clone is supposed to handle halting, I included the
possibility of halting after the final fetch happened and remote refs were
updated (basically anything after resumable download happens). This change
basically says "If the remote ref was not previously written, then write it."
Because the write is all-or-nothing, this means the logic should either write
all of the intended references, or none of them (because they were written
in the previous invocation).
> > +static const char *setup_and_index_pack(const char *filename)
> > +{
> > + const char *primer_idx_path = NULL, *primer_bndl_path = NULL;
> > + primer_idx_path = replace_extension(filename, ".pack", ".idx");
> > + primer_bndl_path = replace_extension(filename, ".pack", ".bndl");
> > +
> > + if (!(primer_idx_path && primer_bndl_path)) {
> > + warning("invalid pack filename '%s', falling back to full "
> > + "clone", filename);
> > + return NULL;
> > + }
> > +
> > + if (!file_exists(primer_bndl_path)) {
> > + if (do_index_pack(filename, primer_idx_path)) {
> > + warning("could not index primer pack, falling back to "
> > + "full clone");
> > + return NULL;
> > + }
> > + }
>
> Can it be another (undetected) failure mode that .bndl somehow
> already existed, but not .idx, leaving the resulting object store in
> an incosistent state? Can do_index_pack() fail and leave .bndl
> behind to get you into such a state?
I don't think so, based on looking at builtin/index-pack.c. write_bundle_file()
happens at the very end of final(), which is long after the process created the
resulting .idx, or has died in the event one is not created. There also is no
automatic cleanup of .idx after that if the process dies somehow after .bndl
is written.
> > +static int write_bundle_refs(const char *bundle_filename)
> > +{
> > + struct ref_transaction *t;
> > + struct bundle_header history_tips;
> > + const char *temp_ref_base = "resume";
> > + struct strbuf err = STRBUF_INIT;
> > + int i;
> > +
> > + init_bundle_header(&history_tips, bundle_filename);
> > + read_bundle_header(&history_tips);
> > +
> > + t = ref_transaction_begin(&err);
> > + for (i = 0; i < history_tips.references.nr; i++) {
> > + struct strbuf ref_name = STRBUF_INIT;
> > + strbuf_addf(&ref_name, "refs/temp/%s/%s/temp-%s",
> > + option_origin, temp_ref_base,
> > + sha1_to_hex(history_tips.references.list[i].sha1));
>
> Can we do this without polluting refs/temp/ namespace?
>
> I am imagining that you are first fetching the .pack file from
> sideways when primer service is available, running index-pack on it
> to produce the bundle, and the step after that is to run "git fetch"
> against the original remote to fill the gap between the bit-stale
> history you got in the bundle and the reality that has progressed
> since the primer pack was made, and you need a way to tell to the
> other end that you already have the history leading to these refs
> when you run "git fetch". I think a bit better way to do so is to
> send these has ".have" while you run the "fetch".
>
> Wouldn't it do if you add the "--advertise-bundle-tips=<bndl>"
> option to "git fetch", move the code to read the bundle header to
> it, and point the bundle's filename with the option when you spawn
> "git fetch"?
My implementation is definitely a minimum working model. I was hoping to
move to something cleaner.
Is this new option preferable to leveraging the "--reference" option you
mentioned in the earlier discussion? I thought that was a clean solution,
especially because it uses an existing option. Additionally, would there be
any use for this new fetch option outside of cloning? If so, I could see the
value--otherwise, knowing that we want to keep as much resume-specific
knowledge inside clone as possible, "--reference" with a .bndl seems better.
- Kevin
^ permalink raw reply
* Re: [PATCH 3/3] docs/cvs-migration: mention cvsimport caveats
From: Jeff King @ 2016-09-28 6:41 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20160928001108.GA9120@thyrsus.com>
On Tue, Sep 27, 2016 at 08:11:08PM -0400, Eric S. Raymond wrote:
> Jeff King <peff@peff.net>:
> > I am not qualified to write on the current state of
> > the art in CVS importing.
>
> I *am* qualified; cvs-fast-export has had a lot of work put into it by
> myself and others over the last five years.
I, for one, would be fine if you wanted to send a patch updating
Documentation/gitcvs-migration.txt to describe cvs-fast-export rather
than cvsimport. I think it would probably even be fine if it mostly just
pointed to an existing guide and said "use this".
-Peff
^ permalink raw reply
* Re: [PATCH 07/11] Resumable clone: add resumable download to http/curl
From: Kevin Wern @ 2016-09-28 6:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kevin Wern, git
In-Reply-To: <xmqqfuozihuv.fsf@gitster.mtv.corp.google.com>
On Fri, Sep 16, 2016 at 03:45:44PM -0700, Junio C Hamano wrote:
> > @@ -1136,7 +1138,10 @@ static int handle_curl_result(struct slot_results *results)
> > curl_easy_strerror(results->curl_result),
> > sizeof(curl_errorstr));
> > #endif
> > - return HTTP_ERROR;
> > + if (results->http_code >= 400)
> > + return HTTP_ERROR;
> > + else
> > + return HTTP_ERROR_RESUMABLE;
> > }
> > }
>
> Hmm, is "anything below 400" a good definition of resumable errors?
Definitely a rough definition, but I think it covers the majority of cases. I
based this on the fact that my example resumable errors (disconnections during
download) either had results->http_code set to 206 or 0, and that http errors
generally indicate some underlying issue that can't be solved with a repeat
request (there are plenty of exceptions, such as 503 or 504 errors that happen
intermittently). I thought that was enough to be a placeholder in the model. I
did consider something like "has (the file changed size or http_code been
206, some indication of success happened) in the last [x] attempts," but I
question the precision of something like that, especially before exploring
other tried-and-true solutions.
This patch is definitely the sketchiest part of my series, IMO. The concept
isn't fleshed out enough, even theoretically--containing only the element of
retrying with some very basic components to those decisions. I think that this
will ultimately require fishing through other examples of resumable downloads
(which was very painful to read when I tried). I know curl has a --retry option,
but doesn't include it in libcurl...harumph. I'll probably try to read that
implementation and chromium's resumable download code again.
I'll get back to you with a better plan here once I read through a few
examples.
- Kevin
^ permalink raw reply
* Re: [PATCH 04/11] Resumable clone: add prime-clone to remote-curl
From: Kevin Wern @ 2016-09-28 6:45 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Kevin Wern, Git Mailing List
In-Reply-To: <CACsJy8C+0M8o15E+iVX+f=izO2CtK7U642NC=XXaG-g41wYAUg@mail.gmail.com>
On Mon, Sep 19, 2016 at 08:52:34PM +0700, Duy Nguyen wrote:
>
> A brief overview for this service in
> Documentation/technical/http-protocol.txt (and maybe
> Documentation/gitremote-helpers.txt as well) would be great help. It's
> a bit hard to follow because at this point I don't know anything about
> the server side (and on top of that I was confused between http
> send/receive vs transport send/receive, but this is my fault).
>
I figured I would miss something in this vein. So many things to cover!
Thanks again for reading.
- Kevin
^ permalink raw reply
* Re: Repeatable Extraction
From: Dennis Kaarsemaker @ 2016-09-28 6:49 UTC (permalink / raw)
To: chris king, git
In-Reply-To: <CAJQwtsidixAAJKp7-b2PmXgs=mS+PbT5ebOmKLJU1nEn7UJ2og@mail.gmail.com>
On di, 2016-09-27 at 20:14 -0700, chris king wrote:
> Hello, first off thanks for such a wonderful tool! I have a general
> question and I hope this is an appropriate spot to ask it.
>
> Is there a way automate extraction that will repeatably generate the
> same files? Currently, each time I extract git portable many of the
> binaries change slightly. For example, if I extract twice using
>
> PortableGit-2.10.0-32-bit.7z.exe -y -gm2
>
> then Beyond Compare tells me that many of the files in usr\bin have
> changed at offset 0x88 and 0x89. Why is that?
Hi Chris,
That file is specific to the git for windows project, not git itself.
While you may be able to find somebody on the git@vger mailinglist who
knows the answer, git for windows also has a separate mailinglist at
https://groups.google.com/forum/#!forum/git-for-windows and a chat room
at https://gitter.im/git-for-windows/git -- these may be of more
assistance in this case.
--
Dennis Kaarsemaker
http://www.kaarsemaker.net
^ permalink raw reply
* Re: thoughts on error passing, was Re: [PATCH 2/2] fsck: handle bad trees like other errors
From: Jeff King @ 2016-09-28 6:54 UTC (permalink / raw)
To: David Turner; +Cc: git, mhagger, David Turner
In-Reply-To: <1475017054.13103.3.camel@frank>
On Tue, Sep 27, 2016 at 06:57:34PM -0400, David Turner wrote:
> > int report_error(struct error_context *err, const char *fmt, ...)
> > {
> > if (err->fn) {
> > va_list ap;
> > va_start(ap, fmt);
> > err->fn(err->data, fmt, ap);
> > va_end(ap);
> > }
> > return -1;
> > }
> >
> > Then low-level functions just take a context and do:
> >
> > return report_error(&err, "some error: %s", foo);
> >
> > And then the callers would pick one of a few generic error contexts:
> >
> > - passing NULL silences the errors
>
> Overall, +1.
>
> I guess I would rather have a sentinel value for silencing errors,
> because I'm worried that someone might read NULL as "don't handle the
> errors, just die". Of course, code review would hopefully catch this,
> but even so, it would be easier to read foo(x, y, silence_errors) than
> foo(x, y, null).
Yeah, I waffled on that. If you look carefully, you'll note that
the report_error() I showed above would actually require such a
"{ NULL, NULL }" global.
I don't plan to make any patches immediately for this, but I'll let it
percolate and consider whether it makes sense to try out for a future
series.
-Peff
^ permalink raw reply
* Re: [PATCH v4 2/2] gitweb: use highlight's shebang detection
From: Ian Kelling @ 2016-09-28 7:37 UTC (permalink / raw)
To: Jakub Narębski, git
In-Reply-To: <c35fc760-ae64-5df3-5985-7eecd06d937f@gmail.com>
On Sun, Sep 25, 2016, at 11:04 AM, Jakub Narębski wrote:
>
> For what it is worth it:
>
> Acked-by: Jakub Narębski <jnareb@gmail.com>
>
> (but unfortunately *not* tested by).
Thank you for all your help.
--
Ian Kelling
^ permalink raw reply
* Re: thoughts on error passing, was Re: [PATCH 2/2] fsck: handle bad trees like other errors
From: Jeff King @ 2016-09-28 8:58 UTC (permalink / raw)
To: Michael Haggerty; +Cc: David Turner, git, David Turner
In-Reply-To: <06b1a3bc-18dc-bd9e-4200-a2eedbec7b97@alum.mit.edu>
On Wed, Sep 28, 2016 at 07:01:38AM +0200, Michael Haggerty wrote:
> > - a global for chaining to error, like:
> >
> > struct error_context print_errors = {
> > error, /* actually a wrapper to handle va_list and NULL data */
> > NULL
> > };
>
> There could also be a global for chaining to `warn()` or `die()`.
I played around a little with this. The latter actually makes a lot of
code cleaner, because we can rely on the functions not returning at all.
So for example, you get:
diff --git a/branch.c b/branch.c
index a5a8dcb..53404b8 100644
--- a/branch.c
+++ b/branch.c
@@ -303,17 +303,13 @@ void create_branch(const char *head,
if (!dont_change_ref) {
struct ref_transaction *transaction;
- struct strbuf err = STRBUF_INIT;
-
- transaction = ref_transaction_begin(&err);
- if (!transaction ||
- ref_transaction_update(transaction, ref.buf,
- sha1, forcing ? NULL : null_sha1,
- 0, msg, &err) ||
- ref_transaction_commit(transaction, &err))
- die("%s", err.buf);
+
+ transaction = ref_transaction_begin(&error_die);
+ ref_transaction_update(transaction, ref.buf,
+ sha1, forcing ? NULL : null_sha1,
+ 0, msg, &error_die);
+ ref_transaction_commit(transaction, &error_die);
ref_transaction_free(transaction);
- strbuf_release(&err);
}
if (real_ref && track)
which is much shorter and to the point (it does rely on the called
functions always calling report_error() and never just returning NULL or
"-1", but that should be the already. If it isn't, we'd be printing
"fatal: " with no message).
Cases that call:
error("%s", err.buf);
can drop the strbuf handling, but of course still need to retain their
conditionals. So they're better, but not as much. I did a half-hearted
conversion of some of the ref code that uses strbufs, and it seems like
it would save a few hundred lines of boilerplate.
There are some cases that are _worse_, because they want to prefix the
error. E.g., in init-db, we have:
struct strbuf err = STRBUF_INIT;
...
if (refs_init_db(&err))
die("failed to set up refs db: %s", err.buf);
which is fairly clean. Using an error_context adds slightly to the
boilerplate:
struct strbuf err_buf = STRBUF_INIT;
struct error_context err = STRBUF_ERR(&err_buf);
...
if (refs_init_db(&err))
die("failed to set up refs db: %s", err_buf.buf);
Though if we wanted to get really magical, the err_buf/err pattern could
be its own single-line macro.
You could solve this more generally with something like:
struct error_prefix_data err;
error_prefix(&err, &error_die, "failed to set up refs db");
refs_init_db(&err.err);
where error_prefix() basically sets us up to call back a function which
concatenates the prefix to the real error, then chains to error_die.
But to cover all cases, error_prefix() would actually have to format the
prefix string. Because some callers would be more like:
error_prefix(&err, &error_print, "unable to frob %s", foo);
do_frob(foo, &err);
We can't just save the va_list passed to error_prefix(), because it's
not valid after we return. So you have to format the prefix into a
buffer, even though in most cases we won't see an error at all (and
doing it completely correctly would involve using a strbuf, which means
there needs to be a cleanup step; yuck).
-Peff
^ permalink raw reply related
* [PATCH 0/3] i-t-a entries in git-status, and git-commit
From: Nguyễn Thái Ngọc Duy @ 2016-09-28 11:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
Let's see if I can get the first commit graduated before it turns two
years old. Six months to go! This series is about some oddities of
intent-to-add entries (aka "git add -N"):
1) An ita entry in git-status shows that it's added in the index
(compared to worktree) as a file change, not new file addition. And
it shows the entry added to HEAD as an empty version. This is due
to an implementation detail that i-t-a entries are registered in
the index as empty blobs with a special flag.
2) When you do a "git commit" with no changes whatsoever, but you
happen to have some ita entries registered, git-commit does not
recognize the resulting commit would be empty and should be caught,
unless --allow-empty is given. This has been reported several times.
3) Same symptom as 2) but with initial commit (diff code is not used
for detecting empty commits this time). If you only have ita
entries in the index by the time you make an initial commit, you'll
create an empty commit even if --allow-empty is not specified.
1) and 2) are fixed by changing the position of ita entries in diff
code. ita entries should be seen as a new file when compared between
worktree and HEAD, and no change when compared between index and HEAD.
Some previous commit made this move globally and was reverted because
it could have dangerous unseen side effects, especially in merge code.
Now we are moving slowly towards that (first patch), this time we try
to handle case by case (e.g. index_differs_from in the second patch).
3) could be handled pretty easily once you know the problem. This is
the third patch.
Nguyễn Thái Ngọc Duy (3):
Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"
diff-lib.c: enable --shift-ita in index_differs_from()
commit: don't be fooled by ita entries when creating initial commit
Documentation/diff-options.txt | 7 +++++++
builtin/commit.c | 11 ++++++++---
cache.h | 1 +
diff-lib.c | 13 +++++++++++++
diff.c | 2 ++
diff.h | 1 +
read-cache.c | 10 ++++++++++
sequencer.c | 5 +++--
t/t2203-add-intent.sh | 41 +++++++++++++++++++++++++++++++++++++++--
t/t7064-wtstatus-pv2.sh | 4 ++--
wt-status.c | 7 ++++++-
11 files changed, 92 insertions(+), 10 deletions(-)
--
2.8.2.524.g6ff3d78
^ permalink raw reply
* [PATCH 1/3] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"
From: Nguyễn Thái Ngọc Duy @ 2016-09-28 11:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160928114348.1470-1-pclouds@gmail.com>
The original commit d95d728aba06a34394d15466045cbdabdada58a2 was
reverted in commit 78cc1a540ba127b13f2f3fd531777b57f3a9cd46 because we
were (and still are) not ready for a new world order. A lot more
investigation must be done to see what is impacted. See the 78cc1a5 for
details.
This patch takes a smaller and safer step. The new behavior is
controlled by shift_ita flag. We can gradually move more diff users to
the new behavior after we are sure it's safe to do so. This flag is
exposed to outside temporarily as "--shift-ita" for people who prefer
"git diff [--cached] --stat" to "git status"
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/diff-options.txt | 7 +++++++
diff-lib.c | 12 ++++++++++++
diff.c | 2 ++
diff.h | 1 +
t/t2203-add-intent.sh | 20 ++++++++++++++++++--
t/t7064-wtstatus-pv2.sh | 4 ++--
wt-status.c | 7 ++++++-
7 files changed, 48 insertions(+), 5 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 7805a0c..e63285c 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -575,5 +575,12 @@ endif::git-format-patch[]
--line-prefix=<prefix>::
Prepend an additional prefix to every line of output.
+--shift-ita::
+ By default entries added by "git add -N" appear as an existing
+ empty file in "git diff" and a new file in "git diff --cached".
+ This option makes the entry appear as a new file in "git diff"
+ and non-existent in "git diff --cached". Experimental option,
+ could be removed in future.
+
For more detailed explanation on these common options, see also
linkgit:gitdiffcore[7].
diff --git a/diff-lib.c b/diff-lib.c
index 3007c85..62d67c8 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -214,6 +214,11 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
!is_null_oid(&ce->oid),
ce->name, 0);
continue;
+ } else if (revs->diffopt.shift_ita && ce_intent_to_add(ce)) {
+ diff_addremove(&revs->diffopt, '+', ce->ce_mode,
+ EMPTY_BLOB_SHA1_BIN, 0,
+ ce->name, 0);
+ continue;
}
changed = match_stat_with_submodule(&revs->diffopt, ce, &st,
@@ -379,6 +384,13 @@ static void do_oneway_diff(struct unpack_trees_options *o,
struct rev_info *revs = o->unpack_data;
int match_missing, cached;
+ /* i-t-a entries do not actually exist in the index */
+ if (revs->diffopt.shift_ita && idx && ce_intent_to_add(idx)) {
+ idx = NULL;
+ if (!tree)
+ return; /* nothing to diff.. */
+ }
+
/* if the entry is not checked out, don't examine work tree */
cached = o->index_only ||
(idx && ((idx->ce_flags & CE_VALID) || ce_skip_worktree(idx)));
diff --git a/diff.c b/diff.c
index c6da383..4178689 100644
--- a/diff.c
+++ b/diff.c
@@ -3923,6 +3923,8 @@ int diff_opt_parse(struct diff_options *options,
return parse_submodule_opt(options, arg);
else if (skip_prefix(arg, "--ws-error-highlight=", &arg))
return parse_ws_error_highlight(options, arg);
+ else if (!strcmp(arg, "--shift-ita"))
+ options->shift_ita = 1;
/* misc options */
else if (!strcmp(arg, "-z"))
diff --git a/diff.h b/diff.h
index ec76a90..5dd4f9c 100644
--- a/diff.h
+++ b/diff.h
@@ -146,6 +146,7 @@ struct diff_options {
int dirstat_permille;
int setup;
int abbrev;
+ int shift_ita;
/* white-space error highlighting */
#define WSEH_NEW 1
#define WSEH_CONTEXT 2
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 8f22c43..c6a4648 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -5,10 +5,24 @@ test_description='Intent to add'
. ./test-lib.sh
test_expect_success 'intent to add' '
+ test_commit 1 &&
+ git rm 1.t &&
+ echo hello >1.t &&
echo hello >file &&
echo hello >elif &&
git add -N file &&
- git add elif
+ git add elif &&
+ git add -N 1.t
+'
+
+test_expect_success 'git status' '
+ git status --porcelain | grep -v actual >actual &&
+ cat >expect <<-\EOF &&
+ DA 1.t
+ A elif
+ A file
+ EOF
+ test_cmp expect actual
'
test_expect_success 'check result of "add -N"' '
@@ -43,7 +57,9 @@ test_expect_success 'i-t-a entry is simply ignored' '
git add -N nitfol &&
git commit -m second &&
test $(git ls-tree HEAD -- nitfol | wc -l) = 0 &&
- test $(git diff --name-only HEAD -- nitfol | wc -l) = 1
+ test $(git diff --name-only HEAD -- nitfol | wc -l) = 1 &&
+ test $(git diff --name-only --shift-ita HEAD -- nitfol | wc -l) = 0 &&
+ test $(git diff --name-only --shift-ita -- nitfol | wc -l) = 1
'
test_expect_success 'can commit with an unrelated i-t-a entry in index' '
diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
index 3012a4d..e319fa2 100755
--- a/t/t7064-wtstatus-pv2.sh
+++ b/t/t7064-wtstatus-pv2.sh
@@ -246,8 +246,8 @@ test_expect_success 'verify --intent-to-add output' '
git add --intent-to-add intent1.add intent2.add &&
cat >expect <<-EOF &&
- 1 AM N... 000000 100644 100644 $_z40 $EMPTY_BLOB intent1.add
- 1 AM N... 000000 100644 100644 $_z40 $EMPTY_BLOB intent2.add
+ 1 .A N... 000000 000000 100644 $_z40 $_z40 intent1.add
+ 1 .A N... 000000 000000 100644 $_z40 $_z40 intent2.add
EOF
git status --porcelain=v2 >actual &&
diff --git a/wt-status.c b/wt-status.c
index 9a14658..5f9b1cd 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -437,7 +437,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
switch (p->status) {
case DIFF_STATUS_ADDED:
- die("BUG: worktree status add???");
+ d->mode_worktree = p->two->mode;
break;
case DIFF_STATUS_DELETED:
@@ -547,6 +547,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
setup_revisions(0, NULL, &rev, NULL);
rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
+ rev.diffopt.shift_ita = 1;
if (!s->show_untracked_files)
DIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
if (s->ignore_submodule_arg) {
@@ -570,6 +571,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)
setup_revisions(0, NULL, &rev, &opt);
DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+ rev.diffopt.shift_ita = 1;
if (s->ignore_submodule_arg) {
handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
} else {
@@ -605,6 +607,8 @@ static void wt_status_collect_changes_initial(struct wt_status *s)
if (!ce_path_match(ce, &s->pathspec, NULL))
continue;
+ if (ce_intent_to_add(ce))
+ continue;
it = string_list_insert(&s->change, ce->name);
d = it->util;
if (!d) {
@@ -911,6 +915,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
init_revisions(&rev, NULL);
DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
+ rev.diffopt.shift_ita = 1;
memset(&opt, 0, sizeof(opt));
opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 2/3] diff-lib.c: enable --shift-ita in index_differs_from()
From: Nguyễn Thái Ngọc Duy @ 2016-09-28 11:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160928114348.1470-1-pclouds@gmail.com>
This function is basically "git diff --cached HEAD", It has three
callers:
- One in builtin/commit.c, which uses it to determine if the index is
different from HEAD and go ahead making a new commit.
- Two in sequencer.c, which use it to see if the index is dirty.
In the first case, if ita entries are present, index_differs_from() may
report "dirty". However at tree creation phase, ita entries are dropped
and the result tree may look exactly the same as HEAD (assuming that
nothing else is changed in index). This is what we need index_differs_from()
for, to catch new empty commits. Enabling shift_ita in index_differs_from()
fixes this.
In the second case, the presence of ita entries are enough to say the
index is dirty and not continue on. Make an explicit check for that
before comparing index against HEAD (whether --shift-ita is present is
irrelevant)
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
cache.h | 1 +
diff-lib.c | 1 +
read-cache.c | 10 ++++++++++
sequencer.c | 5 +++--
t/t2203-add-intent.sh | 11 +++++++++++
5 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index d0494c8..1ddd515 100644
--- a/cache.h
+++ b/cache.h
@@ -561,6 +561,7 @@ extern int do_read_index(struct index_state *istate, const char *path,
extern int read_index_from(struct index_state *, const char *path);
extern int is_index_unborn(struct index_state *);
extern int read_index_unmerged(struct index_state *);
+extern int has_ita_entries(struct index_state *);
#define COMMIT_LOCK (1 << 0)
#define CLOSE_LOCK (1 << 1)
extern int write_locked_index(struct index_state *, struct lock_file *lock, unsigned flags);
diff --git a/diff-lib.c b/diff-lib.c
index 62d67c8..ea55ee2 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -545,6 +545,7 @@ int index_differs_from(const char *def, int diff_flags)
DIFF_OPT_SET(&rev.diffopt, QUICK);
DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
rev.diffopt.flags |= diff_flags;
+ rev.diffopt.shift_ita = 1;
run_diff_index(&rev, 1);
if (rev.pending.alloc)
free(rev.pending.objects);
diff --git a/read-cache.c b/read-cache.c
index 31eddec..f6a5f61 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1674,6 +1674,16 @@ int is_index_unborn(struct index_state *istate)
return (!istate->cache_nr && !istate->timestamp.sec);
}
+int has_ita_entries(struct index_state *istate)
+{
+ int i;
+
+ for (i = 0; i < istate->cache_nr; i++)
+ if (ce_intent_to_add(istate->cache[i]))
+ return 1;
+ return 0;
+}
+
int discard_index(struct index_state *istate)
{
int i;
diff --git a/sequencer.c b/sequencer.c
index eec8a60..10cded0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -469,7 +469,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
unborn = get_sha1("HEAD", head);
if (unborn)
hashcpy(head, EMPTY_TREE_SHA1_BIN);
- if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD", 0))
+ if (has_ita_entries(&the_index) ||
+ index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD", 0))
return error_dirty_index(opts);
}
discard_cache();
@@ -1064,7 +1065,7 @@ static int sequencer_continue(struct replay_opts *opts)
if (ret)
return ret;
}
- if (index_differs_from("HEAD", 0))
+ if (has_ita_entries(&the_index) || index_differs_from("HEAD", 0))
return error_dirty_index(opts);
todo_list = todo_list->next;
return pick_commits(todo_list, opts);
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index c6a4648..aa06415 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -129,5 +129,16 @@ test_expect_success 'cache-tree does skip dir that becomes empty' '
)
'
+test_expect_success 'commit: ita entries ignored in empty commit check' '
+ git init empty-subsequent-commit &&
+ (
+ cd empty-subsequent-commit &&
+ test_commit one &&
+ : >two &&
+ git add -N two &&
+ test_must_fail git commit -m nothing-new-here
+ )
+'
+
test_done
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 3/3] commit: don't be fooled by ita entries when creating initial commit
From: Nguyễn Thái Ngọc Duy @ 2016-09-28 11:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160928114348.1470-1-pclouds@gmail.com>
ita entries are dropped at tree creation phase. If the entire index
consists of just ita entries, the result would be a a commit with no
entries, which should be caught unless --allow-empty is specified.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/commit.c | 11 ++++++++---
t/t2203-add-intent.sh | 10 ++++++++++
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index bb9f79b..56b24cb 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -894,9 +894,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
if (amend)
parent = "HEAD^1";
- if (get_sha1(parent, sha1))
- commitable = !!active_nr;
- else {
+ if (get_sha1(parent, sha1)) {
+ int i, ita_nr = 0;
+
+ for (i = 0; i < active_nr; i++)
+ if (ce_intent_to_add(active_cache[i]))
+ ita_nr++;
+ commitable = active_nr - ita_nr > 0;
+ } else {
/*
* Unless the user did explicitly request a submodule
* ignore mode by passing a command line option we do
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index aa06415..65314fc 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -129,6 +129,16 @@ test_expect_success 'cache-tree does skip dir that becomes empty' '
)
'
+test_expect_success 'commit: ita entries ignored in empty intial commit check' '
+ git init empty-intial-commit &&
+ (
+ cd empty-intial-commit &&
+ : >one &&
+ git add -N one &&
+ test_must_fail git commit -m nothing-new-here
+ )
+'
+
test_expect_success 'commit: ita entries ignored in empty commit check' '
git init empty-subsequent-commit &&
(
--
2.8.2.524.g6ff3d78
^ 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