What's cooking in git.git (Dec 2013, #02; Fri, 6)

What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Junio C Hamano]

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 tip of 'next' has been rewound, ejecting a few topics that
used to be there.

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]

* jn/git-gui-chmod+x (2013-11-25) 1 commit
 - git-gui: chmod +x po2msg, windows/git-gui.sh

 Parked here until I get the same change back from the upstream
 git-gui tree.


* jn/gitk-chmod+x (2013-11-25) 1 commit
 - gitk: chmod +x po2msg

 Parked here until I get the same change back from the upstream gitk
 tree.


* jk/name-pack-after-byte-representation (2013-12-05) 2 commits
 - pack-objects: name pack files after trailer hash
 - sha1write: make buffer const-correct
 (this branch is tangled with jk/pack-bitmap.)


* nd/negative-pathspec (2013-12-06) 3 commits
 - pathspec.c: support adding prefix magic to a pathspec with mnemonic magic
 - Support pathspec magic :(exclude) and its short form :!
 - glossary-content.txt: rephrase magic signature part


* nd/transport-positive-depth-only (2013-12-06) 1 commit
 - clone,fetch: catch non positive --depth option value


* zk/difftool-counts (2013-12-06) 1 commit
 - difftool: display the number of files in the diff queue in the prompt

--------------------------------------------------
[Graduated to "master"]

* ak/submodule-foreach-quoting (2013-09-27) 1 commit
  (merged to 'next' on 2013-10-14 at d77c5f1)
 + submodule foreach: skip eval for more than one argument

 A behavior change, but maybe a worthwhile one: "git submodule
 foreach" was treating its arguments as part of a single command to
 be concatenated and passed to a shell, making writing buggy scripts
 too easy.

 This patch preserves the old "just pass it to the shell" behavior
 when a single argument is passed to 'git submodule foreach' and
 moves to a new "skip the shell and use the arguments passed
 unmolested" behavior when more than one argument is passed.

 The old behavior (always concatenating and passing to the shell)
 was similar to the 'ssh' command, while the new behavior (switching
 on the number of arguments) is what 'xterm -e' does.

 May need more thought to make sure this change is advertised well;
 scripts that used multiple arguments but added their own extra
 layer of quoting are broken, and the users need to adjust them.


* bc/http-100-continue (2013-10-31) 3 commits
  (merged to 'next' on 2013-11-01 at e12ae23)
 + remote-curl: fix large pushes with GSSAPI
 + remote-curl: pass curl slot_results back through run_slot
 + http: return curl's AUTHAVAIL via slot_results

 Issue "100 Continue" responses to help use of GSS-Negotiate
 authentication scheme over HTTP transport when needed.


* jc/bundle (2013-11-12) 1 commit
  (merged to 'next' on 2013-11-21 at 535b046)
 + bundle: use argv-array

 Code clean-up.


* jc/merge-base-reflog (2013-10-29) 2 commits
  (merged to 'next' on 2013-11-01 at 6114764)
 + merge-base: teach "--fork-point" mode
 + merge-base: use OPT_CMDMODE and clarify the command line parsing

 Code the logic in "pull --rebase" that figures out a fork point
 from reflog entries in C.


* jc/ref-excludes (2013-11-01) 5 commits
  (merged to 'next' on 2013-11-04 at fac1ed0)
 + rev-parse: introduce --exclude=<glob> to tame wildcards
 + rev-list --exclude: export add/clear-ref-exclusion and ref-excluded API
 + rev-list --exclude: tests
 + document --exclude option
 + revision: introduce --exclude=<glob> to tame wildcards

 People often wished a way to tell "git log --branches" (and "git
 log --remotes --not --branches") to exclude some local branches
 from the expansion of "--branches" (similarly for "--tags", "--all"
 and "--glob=<pattern>").  Now they have one.


* jh/loose-object-dirs-creation-race (2013-10-28) 1 commit
  (merged to 'next' on 2013-11-01 at 3169b0f)
 + sha1_file.c:create_tmpfile(): Fix race when creating loose object dirs

 Two processes creating loose objects at the same time could have
 failed unnecessarily when they happened to have created objects
 whose names share the same first byte.


* jk/remove-experimental-loose-object-support (2013-11-21) 1 commit
  (merged to 'next' on 2013-11-21 at d37bab7)
 + drop support for "experimental" loose objects

 Read-only support for experimental loose-object format, in which
 users could optionally choose to write in their loose objects for a
 short while between v1.4.3 to v1.5.3 era, has been dropped.


* jk/replace-perl-in-built-scripts (2013-10-29) 1 commit
  (merged to 'next' on 2013-11-01 at 2384e29)
 + use @@PERL@@ in built scripts


* jk/robustify-parse-commit (2013-10-24) 6 commits
  (merged to 'next' on 2013-11-01 at 2bfbaab)
 + checkout: do not die when leaving broken detached HEAD
 + use parse_commit_or_die instead of custom message
 + use parse_commit_or_die instead of segfaulting
 + assume parse_commit checks for NULL commit
 + assume parse_commit checks commit->object.parsed
 + log_tree_diff: die when we fail to parse a commit


* jk/two-way-merge-corner-case-fix (2013-11-04) 3 commits
  (merged to 'next' on 2013-11-04 at 79f4fb0)
 + t1005: add test for "read-tree --reset -u A B"
 + t1005: reindent
 + unpack-trees: fix "read-tree -u --reset A B" with conflicted index

 Fix a rather longstanding corner-case bug in twoway "reset to
 there" merge, which is most often seen in "git am --abort".


* jl/submodule-update-retire-orig-flags (2013-11-11) 1 commit
  (merged to 'next' on 2013-11-13 at 4b70d15)
 + submodule update: remove unnecessary orig_flags variable

 Code clean-up.


* jn/mediawiki-makefile-updates (2013-11-11) 4 commits
  (merged to 'next' on 2013-11-13 at 71c8d20)
 + git-remote-mediawiki build: handle DESTDIR/INSTLIBDIR with whitespace
 + git-remote-mediawiki build: make 'install' command configurable
 + git-remote-mediawiki: honor DESTDIR in "make install"
 + git-remote-mediawiki: do not remove installed files in "clean" target

 Build and installation procedure clean-up.


* jn/perl-lib-extra (2013-11-18) 2 commits
  (merged to 'next' on 2013-11-20 at 8c90afae)
 + Makefile: add PERLLIB_EXTRA variable that adds to default perl path
 + Makefile: rebuild perl scripts when perl paths change

 The new PERLLIB_EXTRA makefile variable can be used to specify
 additional directories Perl modules (e.g. the ones necessary to run
 git-svn) are installed on the platform when building.


* nd/magic-pathspec (2013-11-20) 1 commit
  (merged to 'next' on 2013-11-21 at f914a30)
 + diff: restrict pathspec limitations to diff b/f case only

 "git diff -- ':(icase)makefile'" was unnecessarily rejected at the
 command line parser.

 Needs to be merged to 'maint' later.


* nd/wt-status-align-i18n (2013-11-06) 1 commit
  (merged to 'next' on 2013-11-13 at b033aa0)
 + wt-status: take the alignment burden off translators

 An attempt to automatically align the names in the "git status"
 output, taking the display width of (translated) section labels
 into account.


* nv/parseopt-opt-arg (2013-10-31) 2 commits
  (merged to 'next' on 2013-11-01 at cd2afd9)
 + rev-parse --parseopt: add the --stuck-long mode
 + Use the word 'stuck' instead of 'sticked'

 Enhance "rev-parse --parseopt" mode to help parsing options with
 an optional parameter.


* rh/remote-hg-bzr-updates (2013-11-18) 8 commits
  (merged to 'next' on 2013-11-20 at a36f3c4)
 + remote-bzr, remote-hg: fix email address regular expression
 + test-hg.sh: help user correlate verbose output with email test
 + test-hg.sh: fix duplicate content strings in author tests
 + test-hg.sh: avoid obsolete 'test' syntax
 + test-hg.sh: eliminate 'local' bashism
 + test-bzr.sh, test-hg.sh: prepare for change to push.default=simple
 + test-bzr.sh, test-hg.sh: allow running from any dir
 + test-lib.sh: convert $TEST_DIRECTORY to an absolute path


* rr/for-each-ref-decoration (2013-11-19) 6 commits
  (merged to 'next' on 2013-11-21 at ee7b0ed)
 + for-each-ref: avoid color leakage
 + for-each-ref: introduce %(color:...) for color
 + for-each-ref: introduce %(upstream:track[short])
 + for-each-ref: introduce %(HEAD) asterisk marker
 + t6300 (for-each-ref): don't hardcode SHA-1 hexes
 + t6300 (for-each-ref): clearly demarcate setup

 "git for-each-ref --format=..." learned a few formatting directives;
 e.g. "%(color:red)%(HEAD)%(color:reset) %(refname:short) %(subject)".


* sb/sha1-loose-object-info-check-existence (2013-11-06) 1 commit
  (merged to 'next' on 2013-11-06 at 1ea5b18)
 + sha1_loose_object_info(): do not return success on missing object

 "git cat-file --batch-check=ok" did not check the existence of the
 named object.

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

* fc/transport-helper-fixes (2013-11-13) 12 commits
 - remote-bzr: support the new 'force' option
 - transport-helper: add support to delete branches
 - fast-export: add support to delete refs
 - fast-import: add support to delete refs
 - transport-helper: add support for old:new refspec
 - fast-export: add new --refspec option
 - fast-export: improve argument parsing
 - test-hg.sh: tests are now expected to pass
 - transport-helper: check for 'forced update' message
 - transport-helper: add 'force' to 'export' helpers
 - transport-helper: don't update refs in dry-run
 - transport-helper: mismerge fix

 Updates transport-helper, fast-import and fast-export to allow the
 ref mapping and ref deletion in a way similar to the natively
 supported transports.

 The option name "--refspec" needs to be rethought. It does not mean
 what refspec usually means, even though it shares the same syntax
 with refspec; calling it --refspec only because it shares the same
 syntax is like calling it --asciistring and does not make sense.


* nv/commit-gpgsign-config (2013-11-06) 1 commit
 - Add the commit.gpgsign option to sign all commits

 Introduce commit.gpgsign configuration variable to force every
 commit to be GPG signed.

 Needs tests, perhaps?


* jt/commit-fixes-footer (2013-10-30) 1 commit
 - commit: Add -f, --fixes <commit> option to add Fixes: line

 There is an ongoing discussion around this topic; in general I am
 fairly negative on a new feature that is too narrow and prefer a
 more generic solution that can be tailored for specific needs, as
 many people stated in the thread.

 It appears that the discussion stalled.


* np/pack-v4 (2013-09-18) 90 commits
 . packv4-parse.c: add tree offset caching
 . t1050: replace one instance of show-index with verify-pack
 . index-pack, pack-objects: allow creating .idx v2 with .pack v4
 . unpack-objects: decode v4 trees
 . unpack-objects: allow to save processed bytes to a buffer
 - ...

 Nico and Duy advancing the eternal vaporware pack-v4.  This is here
 primarily for wider distribution of the preview edition.

 Temporarily ejected from 'pu', to try out jk/pack-bitmap, which
 this topic conflicts with.


* jk/pack-bitmap (2013-11-18) 22 commits
 - compat/mingw.h: Fix the MinGW and msvc builds
 - pack-bitmap: implement optional name_hash cache
 - t/perf: add tests for pack bitmaps
 - t: add basic bitmap functionality tests
 - count-objects: recognize .bitmap in garbage-checking
 - repack: consider bitmaps when performing repacks
 - repack: handle optional files created by pack-objects
 - repack: turn exts array into array-of-struct
 - repack: stop using magic number for ARRAY_SIZE(exts)
 - pack-objects: implement bitmap writing
 - rev-list: add bitmap mode to speed up object lists
 - pack-objects: use bitmaps when packing objects
 - pack-bitmap: add support for bitmap indexes
 - documentation: add documentation for the bitmap format
 - ewah: compressed bitmap implementation
 - compat: add endianness helpers
 - sha1_file: export `git_open_noatime`
 - revision: allow setting custom limiter function
 - pack-objects: factor out name_hash
 - pack-objects: refactor the packing list
 - revindex: export new APIs
 - sha1write: make buffer const-correct
 (this branch is tangled with jk/name-pack-after-byte-representation.)

 Borrows the bitmap index into packfiles from JGit to speed up
 enumeration of objects involved in a commit range without having to
 fully traverse the history.


* mf/graph-show-root (2013-10-25) 1 commit
 . graph.c: mark root commit differently

 In a repository with multiple-roots, "log --graph", especially with
 "--oneline", does not give the reader enough visual cue to see
 where one line of history ended and a separate history began.

 This is the version that marks the roots 'x' when they would have
 been marked as '*'; Keshav Kini suggested an alternative of giving
 an extra blank line after every root, which I tend to think is a
 better approach to the problem.


* tg/perf-lib-test-perf-cleanup (2013-09-19) 2 commits
 - perf-lib: add test_perf_cleanup target
 - perf-lib: split starting the test from the execution

 Add test_perf_cleanup shell function to the perf suite, that allows
 the script writers to define a test with a clean-up action.

 Holding until needed.


* yt/shortened-rename (2013-10-18) 2 commits
 - SQUASH??? style fixes and s/omit/shorten/ where appropriate
 - diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible

 Attempts to give more weight on the fact that a filepair represents
 a rename than showing substring of the actual path when diffstat
 lines are not wide enough.

 I am not sure if that is solving a right problem, though.


* rv/send-email-cache-generated-mid (2013-08-21) 2 commits
 - git-send-email: Cache generated message-ids, use them when prompting
 - git-send-email: add optional 'choices' parameter to the ask sub


* rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits
 - ### DONTMERGE: needs better explanation on what config they need
 - pack-refs.c: Add missing call to git_config()
 - show-ref.c: Add missing call to git_config()

 The changes themselves are probably good, but it is unclear what
 basic setting needs to be read for which exact operation.

 Waiting for clarification.
 $gmane/228294


* jc/format-patch (2013-04-22) 2 commits
 - format-patch: --inline-single
 - format-patch: rename "no_inline" field

 A new option to send a single patch to the standard output to be
 appended at the bottom of a message.  I personally have no need for
 this, but it was easy enough to cobble together.  Tests, docs and
 stripping out more MIMEy stuff are left as exercises to interested
 parties.


* jk/gitweb-utf8 (2013-04-08) 4 commits
 - gitweb: Fix broken blob action parameters on blob/commitdiff pages
 - gitweb: Don't append ';js=(0|1)' to external links
 - gitweb: Make feed title valid utf8
 - gitweb: Fix utf8 encoding for blob_plain, blobdiff_plain, commitdiff_plain, and patch

 Various fixes to gitweb.

 Drew Northup volunteered to take a look into this.
 $gmane/226216


* jc/show-branch (2013-06-07) 5 commits
 - show-branch: use commit slab to represent bitflags of arbitrary width
 - show-branch.c: remove "all_mask"
 - show-branch.c: abstract out "flags" operation
 - show-branch.c: lift all_mask/all_revs to a global static
 - show-branch.c: update comment style

 Waiting for the final step to lift the hard-limit before sending it out.

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

* jl/commit-v-strip-marker (2013-12-05) 1 commit
 - commit -v: strip diffs and submodule shortlogs from the commit message


* cl/p4-use-diff-tree (2013-11-22) 1 commit
  (merged to 'next' on 2013-12-06 at fc3c89e)
 + git p4: Use git diff-tree instead of format-patch

 Originally merged to 'next' on 2013-11-27

 Will merge to 'master'.


* jn/scripts-updates (2013-11-26) 7 commits
  (merged to 'next' on 2013-12-06 at 60a7026)
 + remove #!interpreter line from shell libraries
 + test: replace shebangs with descriptions in shell libraries
 + test: make FILEMODE a lazy prereq
 + contrib: remove git-p4import
 + mark contributed hooks executable
 + mark perl test scripts executable
 + mark Windows build scripts executable

 Will merge to 'master'.


* tr/commit-slab-cleanup (2013-12-02) 3 commits
  (merged to 'next' on 2013-12-06 at faee247)
 + commit-slab: sizeof() the right type in xrealloc
 + commit-slab: declare functions "static inline"
 + commit-slab: document clear_$slabname()

 Originally merged to 'next' on 2013-12-02

 Will merge to 'master'.


* tr/doc-git-cherry (2013-11-27) 1 commit
  (merged to 'next' on 2013-12-06 at 9a1ba7a)
 + Documentation: revamp git-cherry(1)

 Originally merged to 'next' on 2013-11-27

 Will merge to 'master'.


* rs/doc-submitting-patches (2013-11-27) 1 commit
  (merged to 'next' on 2013-12-06 at 0628818)
 + SubmittingPatches: document how to handle multiple patches

 Originally merged to 'next' on 2013-11-27

 Will merge to 'master'.


* cc/starts-n-ends-with (2013-12-05) 4 commits
 - replace {pre,suf}fixcmp() with {starts,ends}_with()
 - strbuf: introduce starts_with() and ends_with()
 - builtin/remote: remove postfixcmp() and use suffixcmp() instead
 - environment: normalize use of prefixcmp() by removing " != 0"
 (this branch is used by cc/starts-n-ends-with-endgame; uses jk/remove-deprecated.)

 Remove a few duplicate implementations of prefix/suffix comparison
 functions, and rename them to starts_with and ends_with.

 To avoid unnecessary merge conflicts, this is queued on top of
 jk/remove-deprecated topic.


* cc/starts-n-ends-with-endgame (2013-12-05) 1 commit
 - strbuf: remove prefixcmp() and suffixcmp()
 (this branch uses cc/starts-n-ends-with and jk/remove-deprecated.)

 Endgame for the above topic, that needs to be evil-merged with
 other topics that introduce new uses of prefix/suffix-cmp
 functions.


* jc/push-refmap (2013-12-04) 3 commits
 - push: also use "upstream" mapping when pushing a single ref
 - push: use remote.$name.push as a refmap
 - builtin/push.c: use strbuf instead of manual allocation

 Make "git push origin master" update the same ref that would be
 updated by our 'master' when "git push origin" (no refspecs) is run
 while the 'master' branch is checked out, which makes "git push"
 more symmetric to "git fetch" and more usable for the triangular
 workflow.


* jk/t5000-gzip-simplify (2013-12-04) 1 commit
 - t5000: simplify gzip prerequisite checks

 Test fix.


* js/gnome-keyring (2013-12-04) 1 commit
 - contrib/git-credential-gnome-keyring.c: small stylistic cleanups

 Style fix.


* kn/gitweb-extra-branch-refs (2013-12-04) 4 commits
 - gitweb: Denote non-heads, non-remotes branches
 - gitweb: Add a feature for adding more branch refs
 - gitweb: Return plain booleans in validation methods
 - gitweb: Move check-ref-format code into separate function

 Allow gitweb to be configured to show refs out of refs/heads/ as if
 they were branches.


* mm/mv-file-to-no-such-dir-with-slash (2013-12-04) 1 commit
 - mv: let 'git mv file no-such-dir/' error out


* nd/gettext-vsnprintf (2013-12-04) 1 commit
 - gettext.c: detect the vsnprintf bug at runtime


* tr/send-email-ssl (2013-12-04) 3 commits
 - send-email: set SSL options through IO::Socket::SSL::set_client_defaults
 - send-email: --smtp-ssl-cert-path takes an argument
 - send-email: pass Debug to Net::SMTP::SSL::new


* tb/clone-ssh-with-colon-for-port (2013-12-04) 10 commits
 - git_connect(): use common return point
 - connect.c: refactor url parsing
 - git_connect(): refactor the port handling for ssh
 - git fetch: support host:/~repo
 - t5500: add test cases for diag-url
 - git fetch-pack: add --diag-url
 - git_connect: factor out discovery of the protocol and its parts
 - git_connect: remove artificial limit of a remote command
 - t5601: add tests for ssh
 - t5601: remove clear_ssh, refactor setup_ssh_wrapper


* cn/thin-push-capability (2013-11-25) 1 commit
  (merged to 'next' on 2013-12-06 at a7ae524)
 + send-pack: don't send a thin pack to a server which doesn't support it

 Allow receive-pack to insist on receiving a fat pack from "git
 push" clients.

 Will merge to 'master'.


* jk/remove-deprecated (2013-12-05) 6 commits
  (merged to 'next' on 2013-12-06 at c0c91a2)
 + Sync with 1.8.5
 + stop installing git-tar-tree link
 + peek-remote: remove deprecated alias of ls-remote
 + lost-found: remove deprecated command
 + tar-tree: remove deprecated command
 + repo-config: remove deprecated alias for "git config"
 (this branch is used by cc/starts-n-ends-with and cc/starts-n-ends-with-endgame.)

 Originally merged to 'next' on 2013-12-03

 Will merge to 'master'.


* tr/config-multivalue-lift-max (2013-12-06) 1 commit
  (merged to 'next' on 2013-12-06 at 92afee2)
 + config: arbitrary number of matches for --unset and --replace-all

 Originally merged to 'next' on 2013-11-20

 Will merge to 'master'.


* kb/doc-exclude-directory-semantics (2013-11-07) 1 commit
 - gitignore.txt: clarify recursive nature of excluded directories

 Originally merged to 'next' on 2013-11-13

 Kicked back to 'pu' to replace with a newer reroll ($gmane/237814
 looked OK but there seems to have some loose ends in the
 discussion).


* jc/create-directories-microopt (2013-11-11) 1 commit
 - checkout: most of the time we have good leading directories

 Of unknown value until tested on non-Linux platforms (especially
 Windows).

 Will hold.


* gj/push-more-verbose-advice (2013-11-13) 1 commit
  (merged to 'next' on 2013-12-06 at 574b18a)
 + push: switch default from "matching" to "simple"

 Originally merged to 'next' on 2013-11-21

 Explain 'simple' and 'matching' in "git push" advice message; the
 topmost patch is a rebase of jc/push-2.0-default-to-simple on top
 of it.

 Will cook in 'next'.


* tr/merge-recursive-index-only (2013-10-28) 3 commits
 - merge-recursive: -Xindex-only to leave worktree unchanged
 - merge-recursive: internal flag to avoid touching the worktree
 - merge-recursive: remove dead conditional in update_stages()

 Will hold until using script appears.


* mh/fetch-tags-in-addition-to-normal-refs (2013-10-30) 23 commits
  (merged to 'next' on 2013-12-06 at 3b9c44a)
 + fetch: improve the error messages emitted for conflicting refspecs
 + handle_duplicate(): mark error message for translation
 + ref_remote_duplicates(): extract a function handle_duplicate()
 + ref_remove_duplicates(): simplify loop logic
 + t5536: new test of refspec conflicts when fetching
 + ref_remove_duplicates(): avoid redundant bisection
 + git-fetch.txt: improve description of tag auto-following
 + fetch-options.txt: simplify ifdef/ifndef/endif usage
 + fetch, remote: properly convey --no-prune options to subprocesses
 + builtin/remote.c:update(): use struct argv_array
 + builtin/remote.c: reorder function definitions
 + query_refspecs(): move some constants out of the loop
 + fetch --prune: prune only based on explicit refspecs
 + fetch --tags: fetch tags *in addition to* other stuff
 + fetch: only opportunistically update references based on command line
 + get_expanded_map(): avoid memory leak
 + get_expanded_map(): add docstring
 + builtin/fetch.c: reorder function definitions
 + get_ref_map(): rename local variables
 + api-remote.txt: correct section "struct refspec"
 + t5510: check that "git fetch --prune --tags" does not prune branches
 + t5510: prepare test refs more straightforwardly
 + t5510: use the correct tag name in test

 Originally merged to 'next' on 2013-11-06

 The "--tags" option to "git fetch" used to be literally a synonym to
 a "refs/tags/*:refs/tags/*" refspec, which meant that (1) as an
 explicit refspec given from the command line, it silenced the lazy
 "git fetch" default that is configured, and (2) also as an explicit
 refspec given from the command line, it interacted with "--prune"
 to remove any tag that the remote we are fetching from does not
 have.

 This demotes it to an option; with it, we fetch all tags in
 addition to what would be fetched without the option, and it does
 not interact with the decision "--prune" makes to see what
 remote-tracking refs the local has are missing the remote
 counterpart.

 Will merge to 'master'.


* kb/fast-hashmap (2013-11-18) 14 commits
  (merged to 'next' on 2013-12-06 at f90be3d)
 + read-cache.c: fix memory leaks caused by removed cache entries
 + builtin/update-index.c: cleanup update_one
 + fix 'git update-index --verbose --again' output
 + remove old hash.[ch] implementation
 + name-hash.c: remove cache entries instead of marking them CE_UNHASHED
 + name-hash.c: use new hash map implementation for cache entries
 + name-hash.c: remove unreferenced directory entries
 + name-hash.c: use new hash map implementation for directories
 + diffcore-rename.c: use new hash map implementation
 + diffcore-rename.c: simplify finding exact renames
 + diffcore-rename.c: move code around to prepare for the next patch
 + buitin/describe.c: use new hash map implementation
 + add a hashtable implementation that supports O(1) removal
 + submodule: don't access the .gitmodules cache entry after removing it

 Improvements to our hash table to get it to meet the needs of the
 msysgit fscache project, with some nice performance improvements.

 Will cook in 'next'.


* jn/add-2.0-u-A-sans-pathspec (2013-04-26) 1 commit
  (merged to 'next' on 2013-12-06 at ead2ec8)
 + git add: -u/-A now affects the entire working tree

 Will cook in 'next' until Git 2.0.


* jc/core-checkstat-2.0 (2013-05-06) 1 commit
  (merged to 'next' on 2013-12-06 at ae18007)
 + core.statinfo: remove as promised in Git 2.0

 Will cook in 'next' until Git 2.0.


* jc/push-2.0-default-to-simple (2013-06-18) 1 commit
  (merged to 'next' on 2013-12-06 at 6fad61c)
 + push: switch default from "matching" to "simple"

 Will cook in 'next' until Git 2.0.


* jc/add-2.0-ignore-removal (2013-04-22) 1 commit
  (merged to 'next' on 2013-12-06 at fbaa75a)
 + git add <pathspec>... defaults to "-A"

 Updated endgame for "git add <pathspec>" that defaults to "--all"
 aka "--no-ignore-removal".

 Will cook in 'next' until Git 2.0.


* jc/hold-diff-remove-q-synonym-for-no-deletion (2013-07-19) 1 commit
  (merged to 'next' on 2013-12-06 at 083d67c)
 + diff: remove "diff-files -q" in a version of Git in a distant future

 Will cook in 'next' until a distant future.

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

* aa/transport-non-positive-depth-only (2013-11-26) 1 commit
 . transport: catch non positive --depth option value


* cc/remote-remove-redundant-postfixcmp (2013-11-06) 2 commits
 . Rename suffixcmp() to has_suffix() and invert its result
 . builtin/remote: remove postfixcmp() and use suffixcmp() instead


* th/reflog-annotated-tag (2013-10-28) 1 commit
 . reflog: handle lightweight and annotated tags equally

 "git log -g $annotated_tag", when there is no reflog history, should
 have produced a single output entry (i.e. the ref creation event),
 but instead showed the history leading to the tag.

 Broken at the design level.  Any reflog entry that points at a non
 commit needs to be handled with new code that does not exist yet,
 and lifting the "this code handles only commits" without adding
 such code does not solve anything.

RE: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Felipe Contreras]

Junio C Hamano wrote:
> * fc/transport-helper-fixes (2013-11-13) 12 commits
>  - remote-bzr: support the new 'force' option
>  - transport-helper: add support to delete branches
>  - fast-export: add support to delete refs
>  - fast-import: add support to delete refs
>  - transport-helper: add support for old:new refspec
>  - fast-export: add new --refspec option
>  - fast-export: improve argument parsing
>  - test-hg.sh: tests are now expected to pass
>  - transport-helper: check for 'forced update' message
>  - transport-helper: add 'force' to 'export' helpers
>  - transport-helper: don't update refs in dry-run
>  - transport-helper: mismerge fix
> 
>  Updates transport-helper, fast-import and fast-export to allow the
>  ref mapping and ref deletion in a way similar to the natively
>  supported transports.
> 
>  The option name "--refspec" needs to be rethought. It does not mean
>  what refspec usually means, even though it shares the same syntax
>  with refspec; calling it --refspec only because it shares the same
>  syntax is like calling it --asciistring and does not make sense.

Not true. remote.<name>.fetch is a refspec, and doesn't specify what to fetch,
so does the "refspec" capability in remote helpers, and so does the proposed
--refspec option.

It is exactly what it already means.

-- 
Felipe Contreras

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Thomas Rast]

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

> * jk/pack-bitmap (2013-11-18) 22 commits
>  - compat/mingw.h: Fix the MinGW and msvc builds
>  - pack-bitmap: implement optional name_hash cache
>  - t/perf: add tests for pack bitmaps
>  - t: add basic bitmap functionality tests
>  - count-objects: recognize .bitmap in garbage-checking
>  - repack: consider bitmaps when performing repacks
>  - repack: handle optional files created by pack-objects
>  - repack: turn exts array into array-of-struct
>  - repack: stop using magic number for ARRAY_SIZE(exts)
>  - pack-objects: implement bitmap writing
>  - rev-list: add bitmap mode to speed up object lists
>  - pack-objects: use bitmaps when packing objects
>  - pack-bitmap: add support for bitmap indexes
>  - documentation: add documentation for the bitmap format
>  - ewah: compressed bitmap implementation
>  - compat: add endianness helpers
>  - sha1_file: export `git_open_noatime`
>  - revision: allow setting custom limiter function
>  - pack-objects: factor out name_hash
>  - pack-objects: refactor the packing list
>  - revindex: export new APIs
>  - sha1write: make buffer const-correct
>  (this branch is tangled with jk/name-pack-after-byte-representation.)
>
>  Borrows the bitmap index into packfiles from JGit to speed up
>  enumeration of objects involved in a commit range without having to
>  fully traverse the history.

Peff can decide if he wants to reroll with my nits or not; either way
I'm all for moving it forward and aiming for one of the next releases.

-- 
Thomas Rast
tr@thomasrast.ch

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Matthieu Moy]

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

> * mm/mv-file-to-no-such-dir-with-slash (2013-12-04) 1 commit
>  - mv: let 'git mv file no-such-dir/' error out

Why is it "stalled"? I think I took all remarks into account.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Karsten Blees]

Am 07.12.2013 00:52, schrieb Junio C Hamano:

> * kb/doc-exclude-directory-semantics (2013-11-07) 1 commit
>  - gitignore.txt: clarify recursive nature of excluded directories
> 
>  Originally merged to 'next' on 2013-11-13
> 
>  Kicked back to 'pu' to replace with a newer reroll ($gmane/237814
>  looked OK but there seems to have some loose ends in the
>  discussion).

I'm unaware of any loose ends, could you clarify?

Btw. $gmane/237814 seems to be a different topic, the version in next (and now in pu) was $gmane/237429.


> * kb/fast-hashmap (2013-11-18) 14 commits
>   (merged to 'next' on 2013-12-06 at f90be3d) 

Damn, a day too late :-) I found these two glitches today...is a fixup patch OK or should I do a reroll (or separate patch on top)?

Thanks,
Karsten

--- 8< ---
Subject: [PATCH] fixup! add a hashtable implementation that supports O(1) removal

Use 'unsigned int' for hash-codes everywhere.

Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
memory on 64-bit systems. This is already documented in api-hashmap.txt,
but needs '__attribute__((__packed__))' to work. Reduces e.g.

 struct name_entry {
     struct hashmap_entry ent;
     int namelen;
     char *name;
 };

from 32 to 24 bytes.

Signed-off-by: Karsten Blees <blees@dcon.de>
---
 hashmap.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hashmap.h b/hashmap.h
index f5b3b61..b64567b 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -15,7 +15,7 @@ extern unsigned int memihash(const void *buf, size_t len);
 
 /* data structures */
 
-struct hashmap_entry {
+struct __attribute__((__packed__)) hashmap_entry {
 	struct hashmap_entry *next;
 	unsigned int hash;
 };
@@ -43,7 +43,7 @@ extern void hashmap_free(struct hashmap *map, int free_entries);
 
 /* hashmap_entry functions */
 
-static inline void hashmap_entry_init(void *entry, int hash)
+static inline void hashmap_entry_init(void *entry, unsigned int hash)
 {
 	struct hashmap_entry *e = entry;
 	e->hash = hash;
-- 
1.8.5.1.178.g0a9afc1.dirty

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Felipe Contreras]

On Sat, Dec 7, 2013 at 4:03 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Junio C Hamano wrote:
>> * fc/transport-helper-fixes (2013-11-13) 12 commits
>>  - remote-bzr: support the new 'force' option
>>  - transport-helper: add support to delete branches
>>  - fast-export: add support to delete refs
>>  - fast-import: add support to delete refs
>>  - transport-helper: add support for old:new refspec
>>  - fast-export: add new --refspec option
>>  - fast-export: improve argument parsing
>>  - test-hg.sh: tests are now expected to pass
>>  - transport-helper: check for 'forced update' message
>>  - transport-helper: add 'force' to 'export' helpers
>>  - transport-helper: don't update refs in dry-run
>>  - transport-helper: mismerge fix
>>
>>  Updates transport-helper, fast-import and fast-export to allow the
>>  ref mapping and ref deletion in a way similar to the natively
>>  supported transports.
>>
>>  The option name "--refspec" needs to be rethought. It does not mean
>>  what refspec usually means, even though it shares the same syntax
>>  with refspec; calling it --refspec only because it shares the same
>>  syntax is like calling it --asciistring and does not make sense.
>
> Not true. remote.<name>.fetch is a refspec, and doesn't specify what to fetch,
> so does the "refspec" capability in remote helpers, and so does the proposed
> --refspec option.
>
> It is exactly what it already means.

Not to mention that you can apply the first part of the series without
any problems:

>>  - fast-export: improve argument parsing
>>  - test-hg.sh: tests are now expected to pass
>>  - transport-helper: check for 'forced update' message
>>  - transport-helper: add 'force' to 'export' helpers
>>  - transport-helper: don't update refs in dry-run
>>  - transport-helper: mismerge fix

Even:

>>  - remote-bzr: support the new 'force' option

The rest is for old:new :to-delete refspecs.

To take hostage the whole patch series because of the name --refspec
(which is perfectly fine) is just an excuse to not fix the issues
already present.

-- 
Felipe Contreras

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Thomas Rast]

Karsten Blees <karsten.blees@gmail.com> writes:

> Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
> memory on 64-bit systems. This is already documented in api-hashmap.txt,
> but needs '__attribute__((__packed__))' to work. Reduces e.g.

You'd have to guard __attribute__((__packed__)) with some compiler
detection in git-compat-util.h though.

-- 
Thomas Rast
tr@thomasrast.ch

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Karsten Blees]

Am 07.12.2013 23:23, schrieb Thomas Rast:
> Karsten Blees <karsten.blees@gmail.com> writes:
> 
>> Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
>> memory on 64-bit systems. This is already documented in api-hashmap.txt,
>> but needs '__attribute__((__packed__))' to work. Reduces e.g.
> 
> You'd have to guard __attribute__((__packed__)) with some compiler
> detection in git-compat-util.h though.
> 

Isn't that already handled? __attribute__ is already widely used (e.g. for printf formats), and platforms that don't support it define it as empty (e.g. MSVC). Or do you mean I should account for compiler-specific variants (#pragma pack...)?

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Thomas Rast]

Karsten Blees <karsten.blees@gmail.com> writes:

> Am 07.12.2013 23:23, schrieb Thomas Rast:
>> Karsten Blees <karsten.blees@gmail.com> writes:
>> 
>>> Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
>>> memory on 64-bit systems. This is already documented in api-hashmap.txt,
>>> but needs '__attribute__((__packed__))' to work. Reduces e.g.
>> 
>> You'd have to guard __attribute__((__packed__)) with some compiler
>> detection in git-compat-util.h though.
>> 
>
> Isn't that already handled? __attribute__ is already widely used
> (e.g. for printf formats), and platforms that don't support it define
> it as empty (e.g. MSVC). Or do you mean I should account for
> compiler-specific variants (#pragma pack...)?

True, __attribute__ expands to nothing on unknown compilers, but what
does the compiler do when it sees an unknown attribute?  If some of them
choke, you need a separate macro.

I'm a bit confused myself though, many attributes have special macros in
git-compat-util.h but others we just use in the code.

-- 
Thomas Rast
tr@thomasrast.ch

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Karsten Blees]

Am 08.12.2013 11:20, schrieb Thomas Rast:
> Karsten Blees <karsten.blees@gmail.com> writes:
> 
>> Am 07.12.2013 23:23, schrieb Thomas Rast:
>>> Karsten Blees <karsten.blees@gmail.com> writes:
>>>
>>>> Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
>>>> memory on 64-bit systems. This is already documented in api-hashmap.txt,
>>>> but needs '__attribute__((__packed__))' to work. Reduces e.g.
>>>
>>> You'd have to guard __attribute__((__packed__)) with some compiler
>>> detection in git-compat-util.h though.
>>>
>>
>> Isn't that already handled? __attribute__ is already widely used
>> (e.g. for printf formats), and platforms that don't support it define
>> it as empty (e.g. MSVC). Or do you mean I should account for
>> compiler-specific variants (#pragma pack...)?
> 
> True, __attribute__ expands to nothing on unknown compilers, but what
> does the compiler do when it sees an unknown attribute?  If some of them
> choke, you need a separate macro.
> 
> I'm a bit confused myself though, many attributes have special macros in
> git-compat-util.h but others we just use in the code.
> 

So what do you propose? I basically see three options:

1.) Trial and error

GCC supports __packed__ as of 2.3 (1992), so any other compilers that copied the __attribute__ feature probably won't complain.

2.) Accept the wasted memory

Moving the hash code from the hash table (as in hash.[ch]) to the entry is already a big improvement, as it no longer multiplies hash code + wasted bytes with the load factor. 64-bit software uses more memory in general, so we could just live with it (and only fix the documentation in api-hashmap.txt).

3.) Inject individual fields via macro

Instead of injecting a struct hashmap_entry, which implies alignment to sizeof(struct hashmap_entry), we could inject the individual fields, e.g.

 #define HASHMAP_ENTRY_HEADER struct hashmap_entry __next; unsinged int __hash;

 struct name_entry {
   HASHMAP_ENTRY_HEADER
   int namelen;
   char *name;
 };


What do you think?

Karsten

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Junio C Hamano]

Karsten Blees <karsten.blees@gmail.com> writes:

> Am 07.12.2013 00:52, schrieb Junio C Hamano:
>
>> * kb/doc-exclude-directory-semantics (2013-11-07) 1 commit
>>  - gitignore.txt: clarify recursive nature of excluded directories
>> 
>>  Originally merged to 'next' on 2013-11-13
>> 
>>  Kicked back to 'pu' to replace with a newer reroll ($gmane/237814
>>  looked OK but there seems to have some loose ends in the
>>  discussion).
>
> I'm unaware of any loose ends, could you clarify?
>
> Btw. $gmane/237814 seems to be a different topic, the version in next (and now in pu) was $gmane/237429.

Ah, my mistake. I thought I still had $gmane/237416 and needed to
wait for the discussion to conclude; in fact, a later one in that
thread $gmane/237429 is what came out of that discussion, so this is
good to go, perhaps with an amend with Acked-by: peff.

Thanks.

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Junio C Hamano]

Karsten Blees <karsten.blees@gmail.com> writes:

>> * kb/fast-hashmap (2013-11-18) 14 commits
>>   (merged to 'next' on 2013-12-06 at f90be3d) 
>
> Damn, a day too late :-) I found these two glitches today...is a
> fixup patch OK or should I do a reroll (or separate patch on top)?

A separate patch on top would be the most appropriate.  People have
been looking at the change since mid November, and nobody noticed
the problem; having a separate fix on top is a good way to document
what the specific gotcha that can be easily missed is.

I think the patch you attached describes the issue well, possibly
with a retitle (perhaps "hashmap.h: make sure map entries are
tightly packed", or something.)

Thanks.

> --- 8< ---
> Subject: [PATCH] fixup! add a hashtable implementation that supports O(1) removal
>
> Use 'unsigned int' for hash-codes everywhere.
>
> Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
> memory on 64-bit systems. This is already documented in api-hashmap.txt,
> but needs '__attribute__((__packed__))' to work. Reduces e.g.
>
>  struct name_entry {
>      struct hashmap_entry ent;
>      int namelen;
>      char *name;
>  };
>
> from 32 to 24 bytes.
>
> Signed-off-by: Karsten Blees <blees@dcon.de>
> ---
>  hashmap.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hashmap.h b/hashmap.h
> index f5b3b61..b64567b 100644
> --- a/hashmap.h
> +++ b/hashmap.h
> @@ -15,7 +15,7 @@ extern unsigned int memihash(const void *buf, size_t len);
>  
>  /* data structures */
>  
> -struct hashmap_entry {
> +struct __attribute__((__packed__)) hashmap_entry {
>  	struct hashmap_entry *next;
>  	unsigned int hash;
>  };
> @@ -43,7 +43,7 @@ extern void hashmap_free(struct hashmap *map, int free_entries);
>  
>  /* hashmap_entry functions */
>  
> -static inline void hashmap_entry_init(void *entry, int hash)
> +static inline void hashmap_entry_init(void *entry, unsigned int hash)
>  {
>  	struct hashmap_entry *e = entry;
>  	e->hash = hash;

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Jonathan Nieder]

Karsten Blees wrote:

> GCC supports __packed__ as of 2.3 (1992), so any other compilers
> that copied the __attribute__ feature probably won't complain.

Alas, it looks like HP C doesn't support __packed__ (not that I
care much about HP C):

 http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#Attributes

Maybe a macro expanding to __attribute__((aligned(1))) on the fields
would work?  The same macro could expand to __declspec(align(1)) in
the MSVC build.

Thanks,
Jonathan

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> To take hostage the whole patch series because of the name
> --refspec (which is perfectly fine) is just an excuse to not fix
> the issues already present.

I actually was wondering why you are taking the fixes to the hostage
to force a wrong option name.

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Jeff King]

On Sat, Dec 07, 2013 at 06:03:22PM +0100, Thomas Rast wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > * jk/pack-bitmap (2013-11-18) 22 commits
> [...]
> Peff can decide if he wants to reroll with my nits or not; either way
> I'm all for moving it forward and aiming for one of the next releases.

I'm going to be a bit slow this week, as I'm traveling in China.

I have at least one more local fix queued up (one of the re-rolls
introduced a use-after-free). Your comments make sense to me, though
some of them are "if this is not too hard", and I haven't looked yet to
see how hard some of the requisite refactoring would be. So expect at
least one more re-roll, and I'll try to incorporate your comments.
Thanks for giving it a careful reading.

As an aside, we have been running the last version sent to the list
(modulo the fix I mentioned above) on github.com for a week or two
(previously we were running the old, based-on-v1.8.4 version). So it is
getting exercised.

-Peff

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Sat, Dec 07, 2013 at 06:03:22PM +0100, Thomas Rast wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> > * jk/pack-bitmap (2013-11-18) 22 commits
>> [...]
>> Peff can decide if he wants to reroll with my nits or not; either way
>> I'm all for moving it forward and aiming for one of the next releases.
>
> I'm going to be a bit slow this week, as I'm traveling in China.
>
> I have at least one more local fix queued up (one of the re-rolls
> introduced a use-after-free). Your comments make sense to me, though
> some of them are "if this is not too hard", and I haven't looked yet to
> see how hard some of the requisite refactoring would be. So expect at
> least one more re-roll, and I'll try to incorporate your comments.
> Thanks for giving it a careful reading.
>
> As an aside, we have been running the last version sent to the list
> (modulo the fix I mentioned above) on github.com for a week or two
> (previously we were running the old, based-on-v1.8.4 version). So it is
> getting exercised.

;-).

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Karsten Blees]

Am 09.12.2013 21:08, schrieb Jonathan Nieder:
> Karsten Blees wrote:
> 
>> GCC supports __packed__ as of 2.3 (1992), so any other compilers
>> that copied the __attribute__ feature probably won't complain.
> 
> Alas, it looks like HP C doesn't support __packed__ (not that I
> care much about HP C):
> 
>  http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#Attributes
> 

Thanks for the link

> Maybe a macro expanding to __attribute__((aligned(1))) on the fields
> would work?  The same macro could expand to __declspec(align(1)) in
> the MSVC build.
> 

But alignment is not the same as packing. We still want the structure to be 8-byte aligned (i.e. variables of the type should start at 8-byte boundaries). We just don't want the size of the structure to be padded to a multiple of 8, so that we can extend it without penalty. (Besides, __attribute__((aligned)) / __declspec(align) can only _increase_ the alignment, so aligned(1) would have no effect).

Googling some more, I believe the most protable way to achieve this via 'compiler settings' is

 #pragma pack(push)
 #pragma pack(4)
 struct hashmap_entry {
   struct hashmap_entry *next;
   unsigned int hash;
 };
 #pragma pack(pop)

This is supported by at least GCC, MSVC and HP (see your link). The downside is that we cannot use macros (in git-compat-util.h) to emit #pragmas. But we wouldn't have to, as compilers aren't supposed to barf on unknown #pragmas.


However, considering the portability issues, the macro solution (injecting just the two fields instead of a struct) becomes more and more attractive in my mind...

Karsten

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Jonathan Nieder]

Karsten Blees wrote:

> (Besides, __attribute__((aligned)) / __declspec(align) can only
> _increase_ the alignment, so aligned(1) would have no effect).

Good catch.

> Googling some more, I believe the most protable way to achieve this
> via 'compiler settings' is
>
>  #pragma pack(push)
>  #pragma pack(4)
>  struct hashmap_entry {
>    struct hashmap_entry *next;
>    unsigned int hash;
>  };
>  #pragma pack(pop)
>
> This is supported by at least GCC, MSVC and HP (see your link). The
> downside is that we cannot use macros (in git-compat-util.h) to emit
> #pragmas. But we wouldn't have to, as compilers aren't supposed to
> barf on unknown #pragmas.

Technically this can be done using macros:

 #if (gcc)
 #	define BEGIN_PACKED _Pragma("pack(push,4)")
 #	define END_PACKED _Pragma("pack(pop)")
 #elif (msvc)
 #	define BEGIN_PACKED __pragma(pack(push,4))
 #	define END_PACKED __pragma(pack(pop))
 #else
 	/* Just tolerate a little padding. */
 #	define BEGIN_PACKED
 #	define END_PACKED
 #end

Then you can do:

 BEGIN_PACKED
 struct hashmap_entry {
 	...
 };
 END_PACKED

Whether that's nicer or uglier than the alternatives I leave to you.
;-)

Thanks,
Jonathan

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Karsten Blees]

Am 10.12.2013 00:45, schrieb Jonathan Nieder:
> Karsten Blees wrote:
> 
>> Googling some more, I believe the most protable way to achieve this
>> via 'compiler settings' is
>>
>>  #pragma pack(push)
>>  #pragma pack(4)
>>  struct hashmap_entry {
>>    struct hashmap_entry *next;
>>    unsigned int hash;
>>  };
>>  #pragma pack(pop)
>>
>> This is supported by at least GCC, MSVC and HP (see your link). The
>> downside is that we cannot use macros (in git-compat-util.h) to emit
>> #pragmas. But we wouldn't have to, as compilers aren't supposed to
>> barf on unknown #pragmas.
> 
> Technically this can be done using macros:
> 
>  #if (gcc)
>  #	define BEGIN_PACKED _Pragma("pack(push,4)")
>  #	define END_PACKED _Pragma("pack(pop)")
>  #elif (msvc)
>  #	define BEGIN_PACKED __pragma(pack(push,4))
>  #	define END_PACKED __pragma(pack(pop))
>  #else
>  	/* Just tolerate a little padding. */
>  #	define BEGIN_PACKED
>  #	define END_PACKED
>  #end
> 
> Then you can do:
> 
>  BEGIN_PACKED
>  struct hashmap_entry {
>  	...
>  };
>  END_PACKED
> 

Sorry for the delay...

My intention with #pragma pack was to support as many compilers / platforms as possible (even though I can't test them all). From what I could find on the 'net, support for the _Pragma operator is slim. And as the MSVC build is 32-bit only (AFAIK), this is pretty much a GCC-only solution (and thus equivalent, but much more verbose than the initial __attribute__((__packed__)) variant).

> Whether that's nicer or uglier than the alternatives I leave to you.
> ;-)

If it was really up to me ( :-) ), I'd like to do this:

------ 8< -------
Subject: [PATCH] hashmap.h: make sure map entries are tightly packed

Extending 'struct hashmap_entry' with an int-sized member shouldn't waste
memory on 64-bit systems. This is already documented in api-hashmap.txt,
but struct hashmap_entry needs to be packed for this to work. E.g.

 struct name_entry {
   struct hashmap_entry ent; /* consists of pointer + int */
   int namelen;
   char *name;
 };

should be 24 instead of 32 bytes.

Packing structures is compiler-specific, most compilers support [1]:

 #pragma pack(n) - to set structure packing
 #pragma pack()  - to reset structure packing to default

Compilers are supposed to ignore unknown #pragmas, so using this without
further #if <compiler> guards should be safe.

Wasting a few bytes for padding is acceptable, however, compiling the rest
of git with packed structures (and thus mis-aligned arrays of structs) is
not. Add a test to ensure that '#pragma pack()' really resets the packing.

[1] Compiler docs regarding #pragma pack:
http://gcc.gnu.org/onlinedocs/gcc-4.6.2/gcc/Structure_002dPacking-Pragmas.html#Structure_002dPacking-Pragmas
http://msdn.microsoft.com/en-us/library/2e70t5y1%28v=vs.80%29.aspx
http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#pragma-PACK
http://clang.llvm.org/docs/UsersManual.html#microsoft-extensions
http://uw714doc.sco.com/en/SDK_cprog/_Preprocessing_Directives.html
http://osr507doc.sco.com/en/tools/ANSI_F.3.13_Preprocessing.html
http://docs.oracle.com/cd/E19422-01/819-3690/Pragmas_App.html#73499
http://publib.boulder.ibm.com/infocenter/comphelp/v7v91/index.jsp?topic=%2Fcom.ibm.vacpp7a.doc%2Fcompiler%2Fref%2Frnpgpack.htm

Supposedly wants #pragma pack(0) to reset:
http://techpubs.sgi.com/library/tpl/cgi-bin/getdoc.cgi/0650/bks/SGI_Developer/books/Pragmas/sgi_html/ch04.html

Not supported:
http://software.intel.com/sites/products/documentation/doclib/iss/2013/compiler/cpp-lin/GUID-DD32852C-A0F9-4AC6-BF67-D10D064CC87A.htm

Signed-off-by: Karsten Blees <blees@dcon.de>
---
 hashmap.h          |  7 +++++++
 t/t0011-hashmap.sh |  6 ++++++
 test-hashmap.c     | 17 +++++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/hashmap.h b/hashmap.h
index a816ad4..93b330b 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -15,10 +15,17 @@ extern unsigned int memihash(const void *buf, size_t len);
 
 /* data structures */
 
+/*
+ * Set struct packing to 4 so that we don't waste memory on 64-bit systems.
+ * Struct hashmap_entry is used as prefix to other (un-packed) structures,
+ * so this won't cause alignment issues e.g. for odd elements in an array.
+ */
+#pragma pack(4)
 struct hashmap_entry {
 	struct hashmap_entry *next;
 	unsigned int hash;
 };
+#pragma pack()
 
 typedef int (*hashmap_cmp_fn)(const void *entry, const void *entry_or_key,
 		const void *keydata);
diff --git a/t/t0011-hashmap.sh b/t/t0011-hashmap.sh
index 391e2b6..3f3c90b 100755
--- a/t/t0011-hashmap.sh
+++ b/t/t0011-hashmap.sh
@@ -237,4 +237,10 @@ test_expect_success 'grow / shrink' '
 
 '
 
+test_expect_success '"#pragma pack()" resets packing to default' '
+
+	test_hashmap "pragma-pack" "ok"
+
+'
+
 test_done
diff --git a/test-hashmap.c b/test-hashmap.c
index f5183fb..2d5a63a 100644
--- a/test-hashmap.c
+++ b/test-hashmap.c
@@ -36,6 +36,12 @@ static struct test_entry *alloc_test_entry(int hash, char *key, int klen,
 	return entry;
 }
 
+struct pointer_int
+{
+	void *ptr;
+	int i;
+};
+
 #define HASH_METHOD_FNV 0
 #define HASH_METHOD_I 1
 #define HASH_METHOD_IDIV10 2
@@ -136,6 +142,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds)
  * remove key -> NULL / old value
  * iterate -> key1 value1\nkey2 value2\n...
  * size -> tablesize numentries
+ * pragma-pack -> ok / failure
  *
  * perfhashmap method rounds -> test hashmap.[ch] performance
  */
@@ -239,6 +246,16 @@ int main(int argc, char *argv[])
 			/* print table sizes */
 			printf("%u %u\n", map.tablesize, map.size);
 
+		} else if (!strcmp("pragma-pack", cmd)) {
+
+			if ((sizeof(struct pointer_int) % sizeof(void *)) == 0)
+				printf("ok\n");
+			else
+				printf("sizeof(pointer+int) (%u) is not a "
+				       "multiple of sizeof(pointer) (%u)!\n",
+				       sizeof(struct pointer_int),
+				       sizeof(void *));
+
 		} else if (!strcmp("perfhashmap", cmd) && l1 && l2) {
 
 			perf_hashmap(atoi(p1), atoi(p2));
-- 
1.8.5.1.276.g562b27a

[PATCH] hashmap.h: Use 'unsigned int' for hash-codes everywhere

[Karsten Blees]

Signed-off-by: Karsten Blees <blees@dcon.de>
---

Am 09.12.2013 18:48, schrieb Junio C Hamano:
> Karsten Blees <karsten.blees@gmail.com> writes:
> 
>>> * kb/fast-hashmap (2013-11-18) 14 commits
>>>   (merged to 'next' on 2013-12-06 at f90be3d) 
>>
>> Damn, a day too late :-) I found these two glitches today...is a
>> fixup patch OK or should I do a reroll (or separate patch on top)?
> 
> A separate patch on top would be the most appropriate.

OK, this one's a no-brainer I think. See $gmane/239430 for the latest proposal on the struct packing front.


 Documentation/technical/api-hashmap.txt | 2 +-
 hashmap.h                               | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/technical/api-hashmap.txt b/Documentation/technical/api-hashmap.txt
index b2280f1..42ca234 100644
--- a/Documentation/technical/api-hashmap.txt
+++ b/Documentation/technical/api-hashmap.txt
@@ -80,7 +80,7 @@ prevent expensive resizing. If 0, the table is dynamically resized.
 If `free_entries` is true, each hashmap_entry in the map is freed as well
 (using stdlib's free()).
 
-`void hashmap_entry_init(void *entry, int hash)`::
+`void hashmap_entry_init(void *entry, unsigned int hash)`::
 
 	Initializes a hashmap_entry structure.
 +
diff --git a/hashmap.h b/hashmap.h
index f5b3b61..a816ad4 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -43,7 +43,7 @@ extern void hashmap_free(struct hashmap *map, int free_entries);
 
 /* hashmap_entry functions */
 
-static inline void hashmap_entry_init(void *entry, int hash)
+static inline void hashmap_entry_init(void *entry, unsigned int hash)
 {
 	struct hashmap_entry *e = entry;
 	e->hash = hash;
-- 
1.8.5.1.276.g562b27a

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Karsten Blees]

Am 18.12.2013 14:10, schrieb Karsten Blees:
> +				printf("sizeof(pointer+int) (%u) is not a "
> +				       "multiple of sizeof(pointer) (%u)!\n",
> +				       sizeof(struct pointer_int),
> +				       sizeof(void *));

Should be:
-				       sizeof(struct pointer_int),
-				       sizeof(void *));
+				       (unsigned) sizeof(struct pointer_int),
+				       (unsigned) sizeof(void *));

(sizeof() is 'unsigned int' on 32-bit and 'long unsigned int' on 64-bit; without the cast, "%u" produces warnings on 64-bit and "%lu" on 32-bit...)

Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)

[Karsten Blees]

Am 09.12.2013 15:03, schrieb Karsten Blees:
> 3.) Inject individual fields via macro
> 
> Instead of injecting a struct hashmap_entry, which implies alignment to sizeof(struct hashmap_entry), we could inject the individual fields, e.g.
> 
>  #define HASHMAP_ENTRY_HEADER struct hashmap_entry __next; unsinged int __hash;
> 
>  struct name_entry {
>    HASHMAP_ENTRY_HEADER
>    int namelen;
>    char *name;
>  };
> 

I've tried this as well. However, the change is much more intrusive, and produces lots of strict-aliasing warnings in GCC 4.4, probably due to this bug [1]. So I don't think its a good solution. For anyone interested, the patch can be found at [2].

[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42032
[2] https://github.com/kblees/git/commits/kb/hashmap-v5-fixes-macro

Re: [PATCH] hashmap.h: Use 'unsigned int' for hash-codes everywhere

[Junio C Hamano]

Karsten Blees <karsten.blees@gmail.com> writes:

> OK, this one's a no-brainer I think. See $gmane/239430 for the
> latest proposal on the struct packing front.

Thanks.