New-error-handling #659

merged 175 commits into from May 5, 2012


None yet
8 participants

vmg commented May 3, 2012

We've spent the whole day working on this.

This is the tip of new-error-handling + some custom changes, including fixing the test suite, completely removing the old tests and dropping all the special error codes.

vmg and others added some commits Feb 14, 2012

@vmg vmg error-handling: Add reference documentation 845f831
@vmg vmg error-handling: Add new routines
Obviously all the old throw routines are still in place, so we can
gradually port over.
@vmg vmg refs: Error handling rework. WIP 45d387a
@vmg vmg error-handling: References
Yes, this is error handling solely for `refs.c`, but some of the
abstractions leak all ofer the code base.
@vmg vmg diff: Fix rebase breackage 9d160ba
@vmg vmg error-handling: Repository
This also includes droping `git_buf_lasterror` because it makes no sense
in the new system. Note that in most of the places were it has been
dropped, the code needs cleanup. I.e. GIT_ENOMEM is going away, so
instead it should return a generic `-1` and obviously not throw
@arrbee arrbee Migrating diff to new error handling
Ended up migrating a bunch of upstream functions as well
including vector, attr_file, and odb in order to get this
to work right.
@vmg vmg error-handling: Config e54d8d8
@arrbee arrbee Fix issues raised on pull request
This resolves the comments on pull request #590
@arrbee arrbee Merge pull request #590 from arrbee/new-error-handling
Migrating diff to new error handling
@vmg vmg error-handling: On-disk config file backend

	- Proper error reporting when encountering syntax errors in a
	config file (file, line number, column).

	- Rewritten `config_write`, now with 99% less goto-spaghetti

	- Error state in `git_filebuf`: filebuf write functions no longer
	need to be checked for error returns. If any of the writes performed
	on a buffer fail, the last call to `git_filebuf_commit` or
	`git_filebuf_hash` will fail accordingly and set the appropiate error
	message. Baller!
@arrbee arrbee Migrate ODB files to new error handling
This migrates odb.c, odb_loose.c, odb_pack.c and pack.c to
the new style of error handling.  Also got the unix and win32
versions of map.c.  There are some minor changes to other
files but no others were completely converted.

This also contains an update to filebuf so that a zeroed out
filebuf will not think that the fd (== 0) is actually open
(and inadvertently call close() on fd 0 if cleaned up).

Lastly, this was built and tested on win32 and contains a
bunch of fixes for the win32 build which was pretty broken.
@arrbee arrbee Add map.c with shared p_mmap param validation
Forgot to add this file in the previous commit
@arrbee arrbee Resolve comments from pull request
This converts the map validation function into a macro, tweaks
the GITERR_OS system error automatic appending, and adds a
tentative new error access API and some quick unit tests for
both the old and new error APIs.
@arrbee arrbee Convert attr and other files to new errors
This continues to add other files to the new error handling
style.  I think the only real concerns here are that there are
a couple of error return cases that I have converted to asserts,
but I think that it was the correct thing to do given the new
error style.
@arrbee arrbee Continue error conversion
This converts blob.c, fileops.c, and all of the win32 files.
Also, various minor cleanups throughout the code.  Plus, in
testing the win32 build, I cleaned up a bunch (although not
all) of the warnings with the 64-bit build.
@vmg vmg Merge pull request #595 from arrbee/new-errors-odb
Update odb code to new error handling
@nulltoken nulltoken Make git_path_root() cope with windows network paths 7b93079
@vmg vmg Merge pull request #600 from nulltoken/fix/windows-network-paths
Fix windows network paths
@arrbee arrbee Convert attr, ignore, mwindow, status to new errors
Also cleaned up some previously converted code that still had
little things to polish.
@arrbee arrbee Update to latest clar fd77142
@arrbee arrbee Migrate index, oid, and utils to new errors
This includes a few cleanups that came up while converting
these files.

This commit introduces a could new git error classes, including
the catchall class: GITERR_INVALID which I'm using as the class
for invalid and out of range values which are detected at too low
a level of library to use a higher level classification.  For
example, an overflow error in parsing an integer or a bad letter
in parsing an OID string would generate an error in this class.
@arrbee arrbee Convert indexer, notes, sha1_lookup, and signature
More files moved to new error handling style.
@arrbee arrbee Convert reflog to new errors
Cleaned up some other issues.
@nulltoken nulltoken test_helpers: fix unepextected closing of file on error e285bdc
@arrbee arrbee Merge pull request #602 from arrbee/more-error-handling
More error handling conversions
@arrbee arrbee Reimplment git_status_foreach using git diff
This is an initial reimplementation of status using diff a la
the way that core git does it.
@arrbee arrbee Adding new tests for new status command
This is a work in progress.  This adds two new sets of tests,
the issue_592 tests from @nulltoken's pull request #601 and
some new tests for submodules.  The submodule tests still have
issues where the status is not reported correctly.  That needs
to be fixed before merge.
@arrbee arrbee New status fixes
This adds support for roughly-right tracking of submodules
(although it does not recurse into submodules to detect
internal modifications a la core git), and it adds support
for including unmodified files in diff iteration if requested.
@arrbee arrbee More status testing
This "fixes" the broken t18 status tests to accurately reflect
the new behavior for "created" untracked subdirectories.  See
discussion in the PR for more details.

This also contains the submodules unit test that I forgot to
git add, and ports most of the t18-status.c tests to clar (still
missing a couple of the git_status_file() single file tests).
@arrbee arrbee Migrate remaining status tests to Clar
This finishes up the migration of remaining tests from
tests/t18-status.c over the tests-clar/status/worktree.c.
@arrbee arrbee Merge pull request #607 from nulltoken/fix/file-close-on-error
test helpers: Fix file close on error
@arrbee arrbee Fix crash in new status and add recurse option
This fixes the bug that @nulltoken found (thank you!) where
if there were untracked directories alphabetically after the
last tracked item, the diff implementation would deref a NULL

The fix involved the code which decides if it is necessary
to recurse into a directory in the working dir, so it was
easy to add a new option `GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS`
to control if the contents of untracked directories should be
included in status.
@arrbee arrbee Restore default status recursion behavior
This gives `git_status_foreach()` back its old behavior of
emulating the "--untracked=all" behavior of git.  You can
get any of the various --untracked options by passing flags
to `git_status_foreach_ext()` but the basic version will
keep the behavior it has always had.
@arrbee arrbee Fix error in tree iterator when popping up trees
There was an error in the tree iterator where it would
delete two tree levels instead of just one when popping
up a tree level.  Unfortunately the test data for the
tree iterator did not have any deep trees with subtrees
in the middle of the tree items, so this problem went
unnoticed.  This contains the 1-line fix plus new test
data and tests that reveal the issue.
@arrbee arrbee Eliminate hairy COITERATE macro
I decided that the COITERATE macro was, in the end causing
more confusion that it would save and decided just to write
out the loops that I needed for parallel diff list iteration.
It is not that much code and this just feels less obfuscated.
@arrbee arrbee Fix handling of submodules in trees 277e304
@arrbee arrbee Added submodule API and use in status
When processing status for a newly checked out repo, it is
possible that there will be submodules that have not yet been
initialized.  The only way to distinguish these from untracked
directories is to have some knowledge of submodules.  This
commit adds a new submodule API which, given a name or path,
can determine if it appears to be a submodule and can give
information about the submodule.
@arrbee arrbee Improve config handling for diff,submodules,attrs
This adds support for a bunch of core.* settings that affect
diff and status, plus fixes up some incorrect implementations
of those settings from before.  Also, this cleans up the
handling of config settings in the new submodules code and
in the old attrs/ignore code.
@arrbee arrbee Fix bug when join_n refers to original buffer
There was a bug in git_buf_join_n when the contents of the
original buffer were joined into itself and the realloc
moved the pointer to the original buffer.
@vmg vmg Merge pull request #610 from arrbee/status-rewrite
Rewrite status using diff
@nulltoken nulltoken reference: Fix creation of references with extended ASCII characters …
…in their name
@vmg vmg error-handling: Commit (WIP) 73fe6a8
@vmg vmg Merge pull request #596 from nulltoken/fix/non-7bit-ascii-reference-n…

Allow references to bear non-7bit-ascii names on Windows
@carlosmn carlosmn Clean up valgrind warnings 8e8b6b0
@nulltoken nulltoken Fix MSVC "unreferenced local variable" compilation warning. 17bd6de
@vmg vmg Merge pull request #618 from nulltoken/fix/warning
Fix compilation warning
@nulltoken nulltoken tag: Add git_tag_peel() which recursively peel a tag until a non tag …
…git_object is met
@nulltoken nulltoken transport/local: Fix peeling of nested tags 79fd423
@nulltoken nulltoken Add basic branch management API: git_branch_create(), git_branch_dele…
…te(), git_branch_list()
@nulltoken nulltoken fileops: Make git_futils_mkdir_r() able to skip non-empty directories 555aa45
@nulltoken nulltoken branch: add git_branch_move() 4615f0f
@vmg vmg Merge branch 'new-error-handling' of into …
@nulltoken @vmg nulltoken repository: make git_repository_set_workdir() prettify the path it is…
… being passed
@vmg vmg Merge pull request #619 from nulltoken/topic/branches
Basic branch management API
@carlosmn carlosmn Typedefs don't have enum in front 1a2b872
@nulltoken @vmg nulltoken Fix compilation errors and warnings d4d648b
@carlosmn @vmg carlosmn error-handling: remote, transport 4376f7f
@carlosmn @vmg carlosmn error-handling: protocol, pkt 84d250b
@carlosmn @vmg carlosmn error-handling: git transport 2b386ac
@carlosmn @vmg carlosmn error-handling: http 25530fc
@carlosmn @vmg carlosmn error-handling: netops 56b7df1
@carlosmn @vmg carlosmn netops: show winsock error messages on Windows bd6585a
@carlosmn @vmg carlosmn Use new error handling in the example network code fc1cc20
@carlosmn @vmg carlosmn error-handling: fetch 5eb8aff
@carlosmn @vmg carlosmn error-handling local transport a62053a
@vmg vmg tests: Remove unused file e1f6f94
@vmg vmg status: Remove status_old
This is Git yo. You can fetch stuff from the history if you need it.
@arrbee arrbee Cannot set workdir to a nonexistent dir 1de77cd
@arrbee arrbee Refactor git_repository_open with new options
Add a new command `git_repository_open_ext` with extended options
that control how searching for a repository will be done.  The
existing `git_repository_open` and `git_repository_discover` are
reimplemented on top of it.  We may want to change the default
behavior of `git_repository_open` but this commit does not do that.

Improve support for "gitdir" files where the work dir is separate
from the repo and support for the "separate-git-dir" config.  Also,
add support for opening repos created with `git-new-workdir` script
(although I have only confirmed that they can be opened, not that
all functions work correctly).

There are also a few minor changes that came up:

- Fix `git_path_prettify` to allow in-place prettifying.

- Fix `git_path_root` to support backslashes on Win32.  This fix
  should help many repo open/discover scenarios - it is the one
  function called when opening before prettifying the path.

- Tweak `git_config_get_string` to set the "out" pointer to NULL
  if the config value is not found.  Allows some other cleanup.

- Fix a couple places that should have been calling
  `git_repository_config__weakptr` and were not.

- Fix `cl_git_sandbox_init` clar helper to support bare repos.

nulltoken commented on 1de77cd Apr 11, 2012

@arrbee In the original (less than optimal #622 :) ) proposal, there was a TODO stating "/* TODO: Should we check that the directory actually exists? */".

I wrote it because the tests were passing on Windows (and I was willing to add as little code as possible). However I wasn't able to figure a use case when a missing workdir would be valid.

If the tests fail on *nix (p_realpath?) maybe should we add a #ifdef GIT_WIN32 and ensure git_path_isdir() == true?


nulltoken replied Apr 11, 2012

Or, maybe better, pushing down this test in the win32 posix implementation of p_realpath()?

carlosmn and others added some commits Feb 28, 2012

@carlosmn carlosmn revwalk: allow pushing/hiding a reference by name
The code was already there, so factor it out and let users push an OID
by giving it a reference name. Only refs to commits are
supported. Annotated tags will throw an error.
@carlosmn carlosmn Implement git_merge_base()
It's implemented in revwalk.c so it has access to the revision
walker's commit cache and related functions. The algorithm is the one
used by git, modified so it fits better with the library's functions.
@carlosmn carlosmn revwalk: don't assume malloc succeeds 081d229
@carlosmn carlosmn revwalk: use merge bases to speed up processing
There is no need walk down the parents of a merge base to mark them as
uninteresting because we'll never see them. Calculate the merge bases
in prepare_walk() so mark_uninteresting() can stop at a merge base
instead of walking all the way to the root.
@carlosmn carlosmn revwalk: add test hiding a commit without a merge base
Nothing should be hidden and this shouldn't bother the merge base
@carlosmn carlosmn revwalk: use a priority queue for calculating merge bases
As parents are older than their children, we're appending to the
commit list most of the time, which makes an ordered linked list quite

While we're there, don't sort the results list in the main loop, as
we're sorting them afterwards and it creates extra work.
@carlosmn carlosmn Move git_merge_base() to is own header and document it bf787bd
@carlosmn carlosmn error-handling: revwalk eb8117b
@carlosmn carlosmn branch: plug leaks in git_branch_move() and _delete() 6a8bcfa
@carlosmn carlosmn local transport: plug leak a151569
@vmg vmg Merge pull request #625 from carlosmn/valgrind
Plug a few leaks
@carlosmn carlosmn branch: simplify error handling for git_branch_move()
The cleanup needs to happen anyway, so set the error code and jump
there instead of copying the code.
@carlosmn carlosmn filebuf: add option not to buffer the contents at all
The new indexer needs to be able to bypass any kind of buffering, as
it's trying to map data that it has just written to disk.
@vmg vmg Merge pull request #623 from arrbee/refactor-open
Update git_repository_open
@vmg vmg Merge remote-tracking branch 'carlosmn/revwalk-merge-base' into new-e…
@carlosmn carlosmn pack: signal a short buffer when needed 45d773e
@carlosmn carlosmn Add packfile_unpack_compressed() to the internal header fa67933
@carlosmn carlosmn indexer: start writing the stream indexer
This will allow us to index a packfile as soon as we receive it from
the network as well as storing it with its final name so we don't need
to pass temporary file names around.
@carlosmn carlosmn indexer: Add git_indexer_stream_finalize()
Resolve any lingering deltas, write out the index file and rename the
@carlosmn carlosmn examples: stream indexer example 907ebe8
@carlosmn carlosmn indexer: add git_indexer_stream_free() and _hash() 1c9c081
@arrbee arrbee Add support for pathspec to diff and status
This adds preliminary support for pathspecs to diff and status.
The implementation is not very optimized (it still looks at
every single file and evaluated the the pathspec match against
them), but it works.
@vmg vmg Merge pull request #627 from arrbee/diff-with-pathspec
Diff with pathspec
@nulltoken nulltoken Fix MSVC compilation warnings
Removed unreferenced variables.
@nulltoken nulltoken Fix git_repository_set_index() refcount issue
git_repository_free() calls git_index_free() if the owned index is not null.

According to the doc, when setting a new index through git_repository_set_index() the caller has still to take care of releasing the index by itself.

In order to cope with this, this fix makes sure the index refcount is incremented when a new repository is being plugged a new index.
@carlosmn carlosmn repo: plug a couple of leaks 146f5c7
schu tests-clar/repo: fix unused warning
ifdef GIT_WIN32 helper unposix_path() to avoid unused-function warning
on non-Windows systems.

Signed-off-by: schu <>
@carlosmn carlosmn config: parse quoted values
Variable values may be quoted to include newlines, literal quotes and
other characters. Add support for these and test it.
@arrbee arrbee Merge pull request #599 from carlosmn/config-quotes
Support config value quoting
@arrbee arrbee Update clar and remove old helpers
This updates to the latest clar which includes the helpers
`cl_assert_equal_s` and `cl_assert_equal_i`.  Convert the code
over to use those and remove the old libgit2-only helpers.
@arrbee arrbee Remove old status implementation
This removes the code for the old status implementation.
@arrbee arrbee Add git_reference_lookup_oid and lookup_resolved
Adds a new public reference function `git_reference_lookup_oid`
that directly resolved a reference name to an OID without returning
the intermediate `git_reference` object (hence, no free needed).

Internally, this adds a `git_reference_lookup_resolved` function
that combines looking up and resolving a reference.  This allows
us to be more efficient with memory reallocation.

The existing `git_reference_lookup` and `git_reference_resolve`
are reimplmented on top of the new utility and a few places in the
code are changed to use one of the two new functions.
@arrbee arrbee Fix warnings on 64-bit windows builds
This fixes all the warnings on win64 except those in deps, which
come from the regex code.
@vmg vmg Merge pull request #629 from nulltoken/issue/index-refcount
Index refcount issue
@vmg vmg Merge remote-tracking branch 'carlosmn/indexer-stream' into new-error…
@carlosmn carlosmn tree-cache: don't error out on a childless invalidated entry
The code used to assume that there had to be data after the newline in
a tree cache extension entry. This isn't true for a childless
invalidated entry if it's the last one, as there won't be any children
nor a hash to take up space.

Adapt the off-by-one comparison to also work in this case. Fixes #633.
schu tests-clar: update to latest version of clar
Signed-off-by: schu <>
@nulltoken nulltoken Fix git_repository_set_odb() refcount issue
git_repository_free() calls git_odb_free() if the owned odb is not null.

According to the doc, when setting a new odb through git_repository_set_odb() the caller has to take care of releasing the odb by himself.
@arrbee arrbee Rename to git_reference_name_to_oid 26515e7
@vmg vmg Merge pull request #637 from nulltoken/issue/odb-refcount
Fix git_repository_set_odb() refcount issue
@vmg vmg Merge pull request #632 from arrbee/win64-cleanup
Code clean up, including fixing warnings on Windows 64-bit build
@carlosmn carlosmn fetch: use the streaming indexer when downloading a pack
This changes the git_remote_download() API, but the existing one is
silly, so you don't get to complain.

The new API allows to know how much data has been downloaded, how many
objects we expect in total and how many we've processed.
@carlosmn carlosmn examples: port 'fetch' to the new API db0f96a
@carlosmn carlosmn transports: buffer the git requests before sending them
Trying to send every single line immediately won't give us any speed
improvement and duplicates the code we need for other transports. Make
the git transport use the same buffer functions as HTTP.
@carlosmn carlosmn examples: run fetch in a background thread
This allows us to give updates on how it's doing
@carlosmn carlosmn revwalk: return GIT_EREVWALKER earlier if no references were pushed
In the case that walk->one is NULL, we know that we have no positive
references, so we already know that the revwalk is over.
@carlosmn carlosmn remote: run a callback when updating the branch tips
This allows the caller to update an internal structure or update the
user output with the tips that were updated.

While in the area, only try to update the ref if the value is
different from its old one.
@carlosmn carlosmn config: also allow escaping outside of a quoted string
This limitation was a misparsing of the documentation.
@arrbee arrbee Implement git_pool paged memory allocator
This adds a `git_pool` object that can do simple paged memory
allocation with free for the entire pool at once.  Using this,
you can replace many small allocations with large blocks that
can then cheaply be doled out in small pieces.  This is best
used when you plan to free the small blocks all at once - for
example, if they represent the parsed state from a file or data
stream that are either all kept or all discarded.

There are two real patterns of usage for `git_pools`: either
for "string" allocation, where the item size is a single byte
and you end up just packing the allocations in together, or for
"fixed size" allocation where you are allocating a large object
(e.g. a `git_oid`) and you generally just allocation single
objects that can be tightly packed.  Of course, you can use it
for other things, but those two cases are the easiest.
@arrbee arrbee Convert attrs and diffs to use string pools
This converts the git attr related code (including ignores) and
the git diff related code (and implicitly the status code) to use
`git_pools` for storing strings.  This reduces the number of small
blocks allocated dramatically.
@arrbee arrbee Convert revwalk to use git_pool
This removes the custom paged allocator from revwalk and
replaces it with a `git_pool`.
@arrbee arrbee Moving power-of-two bit utilities into util.h 25f258e
@arrbee arrbee Adding stash to hashtable implementation
Adding a small stash of nodes with key conflicts has been
demonstrated to greatly increase the efficiency of a cuckoo
hashtable.  See:

for more details.
@arrbee arrbee Import khash.h from attractivechaos/klib ada488b
@arrbee arrbee Convert hashtable usage over to khash
This updates khash.h with some extra features (like error checking
on allocations, ability to use wrapped malloc, foreach calls, etc),
creates two high-level wrappers around khash: `git_khash_str` and
`git_khash_oid` for string-to-void-ptr and oid-to-void-ptr tables,
then converts all of the old usage of `git_hashtable` over to use
these new hashtables.

For `git_khash_str`, I've tried to create a set of macros that
yield an API not too unlike the old `git_hashtable` API.  Since
the oid hashtable is only used in one file, I haven't bother to
set up all those macros and just use the khash APIs directly for
@carlosmn carlosmn Include the new config test file 19dd4e2
@vmg vmg Merge pull request #641 from carlosmn/networking
More Networking updates
@arrbee arrbee Rename git_khash_str to git_strmap, etc.
This renamed `git_khash_str` to `git_strmap`, `git_hash_oid` to
`git_oidmap`, and deletes `git_hashtable` from the tree, plus
adds unit tests for `git_strmap`.
@arrbee arrbee Merge pull request #642 from arrbee/mem-pools
Memory pools and khash hashtables
@nulltoken @arrbee nulltoken diff: fix generation of the header of a removal patch eb3d71a
@carlosmn carlosmn error handling: move the missing parts over to the new error handling 3aa351e
@arrbee arrbee Fix leading slash behavior in attrs/ignores
We were not following the git behavior for leading slashes
in path names when matching git ignores and git attribute
file patterns.  This should fix issue #638.
@vmg vmg Merge pull request #646 from arrbee/ignore-pat-leading-slash
Ignore pat leading slash
@arrbee arrbee Fix Win32 warnings 821f6bc
@schu schu refs: fix unused-but-set warning 9738e2c
@schu schu remote: add more doc on git_remote_free 8af503b
@carlosmn carlosmn pkt: bring back GIT_ESHORTBUFFER
The recent 64-bit Windows fixes changed the return code in
git_pkt_parse_line() so it wouldn't signal a short buffer, breaking
the network code. Bring it back.
@carlosmn carlosmn Add a travis config file
Teach travis how to build the project.
@nulltoken nulltoken buf: add git_buf_len() accessor to expose the current length of the b…
…uffer content
@nulltoken nulltoken diff: provide more context to the consumer of the callbacks
Update the callback to provide some information related to the file change being processed and the range of the hunk, when applicable.
@nulltoken nulltoken buf: deploy git_buf_len() fa6420f
@nulltoken nulltoken Leverage GIT_UNUSED macro to explicitly mark a function parameter as …
…purposely unused
@carlosmn carlosmn net: recognize and report server-side error messages
When e.g. a repository isn't found, the server sends an error saying
so. Put that error message in our error buffer.
@arrbee arrbee Merge pull request #652 from nulltoken/topic/diff-callbacks
diff: provide more context to the consumer of the callbacks
@vmg vmg Merge pull request #654 from carlosmn/pkt-err
Recognize and report server-side error messages
@nulltoken nulltoken attr: add test coverage related to crlf normalization while staging 4e7a3c7
@vmg vmg Merge pull request #655 from nulltoken/topic/cr-normalization-test
crlf normalization test coverage
@schu schu tests-clar/diff: mark output_len unused 52877c8

nulltoken commented on 52877c8 May 1, 2012

@schu Thanks!

schu and others added some commits May 1, 2012

@schu schu remote: don't free transport on disconnect
Currently, git_remote_disconnect not only closes the connection but also
frees the underlying transport object, making it impossible to write
code like

	// fetch stuff

	// close connection

	// call user provided callback for each ref
	git_remote_update_tips(remote, callback)

because remote->refs points to references owned by the transport object.
This means, we have an idling connection while running the callback for
each reference.

Instead, allow immediate disconnect and free the transport later in
@vmg vmg Merge pull request #658 from schu/remote-disconnect
remote: don't free transport on disconnect
@vmg vmg Merge remote-tracking branch 'carlosmn/remaining-errors' into new-err…

@arrbee @arrbee arrbee Fix usage of "new" for fieldname in public header
This should restore the ability to include libgit2 headers
in C++ projects.

Cherry picked 2de6020 from
development into new-error-handling.
@arrbee @arrbee arrbee Copy values to avoid strict aliasing warning
To make this code more resilient to future changes, we'll
explicitly translate the libgit2 structure to the libxdiff
@arrbee @arrbee arrbee Convert from strnlen to git_text_is_binary
Since strnlen is not supported on all platforms and since we
now have the shiny new git_text_is_binary in the filtering
code, let's convert diff binary detection to use the new stuff.
@vmg vmg Merge branch 'new-error-handling' into development
@vmg vmg Update test suite 946a6dc
@vmg vmg Move test resources 8c83fea
@vmg vmg Backport more test data a1d0802
@arrbee arrbee chmod for writability when writing test files 17f1c9f
@vmg vmg Boom b02bcd9
@vmg vmg Remove old and unused error codes 3fbcac8

This pull request fails (merged 3fbcac8 into 2218fd5).


vmg commented May 3, 2012

Travis complains about strncat. I complain about somebody adding strncat in the code base. We have buf functions for a reason!


nulltoken commented May 3, 2012

We've spent the whole day working on this.

Amazing job!

Beside #660, everything looks ok on Windows. All tests pass.


nulltoken commented May 3, 2012

Travis complains about strncat. I complain about somebody adding strncat in the code base. We have buf functions for a reason!

@tanoku As strncat is being used in an error handling mechanism, it might be safer to rely on a statically allocated char array rather than using dynamic git_buf functions. This would avoid having to deal with potential memory related issues. I've added a commit to #660 which should fix the usage of strncat.

However, as Travis doesn't monitor development-merge branch, I cannot be sure it doesn't complain any longer.

This pull request fails (merged 2277216 into 2218fd5).


vmg commented May 3, 2012

@nulltoken: I've just pushed a commit that uses git_buf inside of error handling. I'm OK with it -- the only errors that can be raised are OOM ones.

vmg and others added some commits May 3, 2012

@vmg vmg Silence return value warning 76873c0
@schu schu notes: honor core.notesRef
Setting core.notesRef allows to change the default notes reference used
by Git. Check if set before using GIT_NOTES_DEFAULT_REF. Fixes #649.
@schu schu notes: add git_note_default_ref()
Add git_note_default_ref to allow easy retrieval of the currently set
default notes reference.

This pull request fails (merged 76873c0 into 2218fd5).

arrbee added some commits May 3, 2012

@arrbee arrbee Support reading attributes from index
Depending on the operation, we need to consider gitattributes
in both the work dir and the index.  This adds a parameter to
all of the gitattributes related functions that allows user
control of attribute reading behavior (i.e. prefer workdir,
prefer index, only use index).

This fix also covers allowing us to check attributes (and
hence do diff and status) on bare repositories.

This was a somewhat larger change that I hoped because it had
to change the cache key used for gitattributes files.
@arrbee arrbee Fix memory leaks and use after free b709e95

nulltoken commented on src/buffer.c in baaf1c4 May 4, 2012

@tanoku 🔥 va_copy doesn't look valid on Windows

2>....\libgit2\src\buffer.c(157): warning C4013: 'va_copy' undefined; assuming extern returning int


peff replied May 4, 2012

We had to deal with this in git.git. You can find our portability setup here, which has been working for a few years (and certainly works on windows):


nulltoken replied May 5, 2012

@peff I think @tanoku already found the source-of-all-wonders (cf. 1adf8c6) :)

This pull request fails (merged 1adf8c6 into 2218fd5).

This pull request passes (merged 674a198 into 2218fd5).

arrbee and others added some commits May 4, 2012

@arrbee arrbee Fix valgrind issues
There are three changes here:
- correctly propogate error code from failed object lookups
- make zlib inflate use our allocators
- add OID to notfound error in ODB lookups
@vmg vmg Merge pull request #663 from schu/notes-honor-config
Honor core.notesRef config option

This pull request passes (merged d3a60dc into 2218fd5).

This pull request passes (merged f95e8cc into 2218fd5).

@vmg vmg Merge pull request #664 from arrbee/attrs-from-index
Support git attrs from index (and bare repo)

vmg commented May 5, 2012


@vmg vmg added a commit that referenced this pull request May 5, 2012

@vmg vmg Merge pull request #659 from libgit2/development-merge

@vmg vmg merged commit 48ecd12 into development May 5, 2012

This pull request passes (merged 4ef14af into 2218fd5).

juzna commented on 3fbcac8 May 8, 2012

This breaks php-git build :(


vmg replied May 8, 2012

This will break the error handling in many of the libgit2 wrappers. The fix should be rather straightforward, though, and the results are worth it.


carlosmn replied May 8, 2012

Breaks how? Does it cause runtime problems? Can't it build? If it's just a matter of being out of sync, then it's a matter of updating php-git.

juzna replied May 8, 2012

Yep, php-git won't build because it still has references to the removed constants. I'll add an issue there to update.
(sorry for such stupid report, I was already too tired yesterday).

@phatblat phatblat pushed a commit to phatblat/libgit2 that referenced this pull request Sep 13, 2014

@vmg vmg Merge pull request #659 from libgit2/development-merge

nulltoken deleted the development-merge branch Nov 8, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment