"make check-docs", "git help -a", etc. did not account for cases
where a particular build may deliberately omit some subcommands,
which has been corrected.
* js/misc-doc-fixes:
Turn `git serve` into a test helper
test-tool: handle the `-C <directory>` option just like `git`
check-docs: do not bother checking for legacy scripts' documentation
docs: exclude documentation for commands that have been excluded
check-docs: allow command-list.txt to contain excluded commands
help -a: do not list commands that are excluded from the build
Makefile: drop the NO_INSTALL variable
remote-testgit: move it into the support directory for t5801
%(push:track) token used in the --format option to "git
for-each-ref" and friends was not showing the right branch, which
has been fixed.
* dr/ref-filter-push-track-fix:
ref-filter: use correct branch for %(push:track)
An earlier update for MinGW and Cygwin accidentally broke MSVC build,
which has been fixed.
* ss/msvc-path-utils-fix:
MSVC: include compat/win32/path-utils.h for MSVC, too, for real_path()
"git clone" learned a new --server-option option when talking over
the protocol version 2.
* jt/clone-server-option:
clone: send server options when using protocol v2
transport: die if server options are unsupported
Code tightening against a "wrong" object appearing where an object
of a different type is expected, instead of blindly assuming that
the connection between objects are correctly made.
* tb/unexpected:
rev-list: detect broken root trees
rev-list: let traversal die when --missing is not in use
get_commit_tree(): return NULL for broken tree
list-objects.c: handle unexpected non-tree entries
list-objects.c: handle unexpected non-blob entries
t: introduce tests for unexpected object types
t: move 'hex2oct' into test-lib-functions.sh
Further code clean-up to allow the lowest level of name-to-object
mapping layer to work with a passed-in repository other than the
default one.
* nd/sha1-name-c-wo-the-repository: (34 commits)
sha1-name.c: remove the_repo from get_oid_mb()
sha1-name.c: remove the_repo from other get_oid_*
sha1-name.c: remove the_repo from maybe_die_on_misspelt_object_name
submodule-config.c: use repo_get_oid for reading .gitmodules
sha1-name.c: add repo_get_oid()
sha1-name.c: remove the_repo from get_oid_with_context_1()
sha1-name.c: remove the_repo from resolve_relative_path()
sha1-name.c: remove the_repo from diagnose_invalid_index_path()
sha1-name.c: remove the_repo from handle_one_ref()
sha1-name.c: remove the_repo from get_oid_1()
sha1-name.c: remove the_repo from get_oid_basic()
sha1-name.c: remove the_repo from get_describe_name()
sha1-name.c: remove the_repo from get_oid_oneline()
sha1-name.c: add repo_interpret_branch_name()
sha1-name.c: remove the_repo from interpret_branch_mark()
sha1-name.c: remove the_repo from interpret_nth_prior_checkout()
sha1-name.c: remove the_repo from get_short_oid()
sha1-name.c: add repo_for_each_abbrev()
sha1-name.c: store and use repo in struct disambiguate_state
sha1-name.c: add repo_find_unique_abbrev_r()
...
When given a tag that points at a commit-ish, "git replace --graft"
failed to peel the tag before writing a replace ref, which did not
make sense because the old graft mechanism the feature wants to
mimick only allowed to replace one commit object with another.
This has been fixed.
* cc/replace-graft-peel-tags:
replace: peel tag when passing a tag first to --graft
replace: peel tag when passing a tag as parent to --graft
t6050: redirect expected error output to a file
t6050: use test_line_count instead of wc -l
The trace2 tracing facility learned to auto-generate a filename
when told to log to a directory.
* js/trace2-to-directory:
trace2: write to directory targets
The list of conflicted paths shown in the editor while concluding a
conflicted merge was shown above the scissors line when the
clean-up mode is set to "scissors", even though it was commented
out just like the list of updated paths and other information to
help the user explain the merge better.
* dl/merge-cleanup-scissors-fix:
cherry-pick/revert: add scissors line on merge conflict
sequencer.c: save and restore cleanup mode
merge: add scissors line on merge conflict
merge: cleanup messages like commit
parse-options.h: extract common --cleanup option
commit: extract cleanup_mode functions to sequencer
t7502: clean up style
t7604: clean up style
t3507: clean up style
t7600: clean up style
"git cherry-pick" run with the "-x" or the "--signoff" option used
to (and more importantly, ought to) clean up the commit log message
with the --cleanup=space option by default, but this has been
broken since late 2017. This has been fixed.
* pw/sequencer-cleanup-with-signoff-x-fix:
sequencer: fix cleanup with --signoff and -x
The "git pack-objects" command learned to report the number of
objects it packed via the trace2 mechanism.
* jk/pack-objects-reports-num-objects-to-trace2:
pack-objects: write objects packed to trace2
Running "git add" on a repository created inside the current
repository is an explicit indication that the user wants to add it
as a submodule, but when the HEAD of the inner repository is on an
unborn branch, it cannot be added as a submodule. Worse, the files
in its working tree can be added as if they are a part of the outer
repository, which is not what the user wants. These problems are
being addressed.
* km/empty-repo-is-still-a-repo:
add: error appropriately on repository with no commits
dir: do not traverse repositories with no commits
submodule: refuse to add repository with no commits
"git tag" learned to give an advice suggesting it might be a
mistake when creating an annotated or signed tag that points at
another tag.
* dl/warn-tagging-a-tag:
tag: advise on nested tags
tag: fix formatting
"git merge-recursive" backend recently learned a new heuristics to
infer file movement based on how other files in the same directory
moved. As this is inherently less robust heuristics than the one
based on the content similarity of the file itself (rather than
based on what its neighbours are doing), it sometimes gives an
outcome unexpected by the end users. This has been toned down to
leave the renamed paths in higher/conflicted stages in the index so
that the user can examine and confirm the result.
* en/merge-directory-renames:
merge-recursive: switch directory rename detection default
merge-recursive: give callers of handle_content_merge() access to contents
merge-recursive: track information associated with directory renames
t6043: fix copied test description to match its purpose
merge-recursive: switch from (oid,mode) pairs to a diff_filespec
merge-recursive: cleanup handle_rename_* function signatures
merge-recursive: track branch where rename occurred in rename struct
merge-recursive: remove ren[12]_other fields from rename_conflict_info
merge-recursive: shrink rename_conflict_info
merge-recursive: move some struct declarations together
merge-recursive: use 'ci' for rename_conflict_info variable name
merge-recursive: rename locals 'o' and 'a' to 'obuf' and 'abuf'
merge-recursive: rename diff_filespec 'one' to 'o'
merge-recursive: rename merge_options argument from 'o' to 'opt'
Use 'unsigned short' for mode, like diff_filespec does
This is a complete line. We're not expecting the next function to add
anything to the same line.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If you have the memory to handle it, the ideal case is to run a single
spatch invocation with all of the source files. But the only way to do
so now is to pick an arbitrarily large batch size. Let's make "0" do
this, which is a little friendlier (and doesn't otherwise have a useful
meaning).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In Git for Windows, we use the MSYS2 Bash which inherits a non-standard
PID model from Cygwin's POSIX emulation layer: every MSYS2 process has a
regular Windows PID, and in addition it has an MSYS2 PID (which
corresponds to a shadow process that emulates Unix-style signal
handling).
With the upgrade to the MSYS2 runtime v3.x, this shadow process cannot
be accessed via `OpenProcess()` any longer, and therefore t6500 thought
incorrectly that the process referenced in `gc.pid` (which is not
actually a real `gc` process in this context, but the current shell) no
longer exists.
Let's fix this by making sure that the Windows PID is written into
`gc.pid` in this test script so that `git.exe` is able to understand
that that process does indeed still exist.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Just as we instruct Apache to pass through ASAN_OPTIONS (so that
server-side Git programs it spawns will respect our options while
running the tests), we should do the same with LSAN_OPTIONS. Otherwise
trying to collect a list of leaks like:
export LSAN_OPTIONS=exitcode=0:log_path=/tmp/lsan
make SANITIZE=leak test
won't work for http tests (the server-side programs won't log their
leaks to the right place, and they'll prematurely die, producing a
spurious test failure).
Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The fread/fwrite-like functions in http.c, namely fread_buffer,
fwrite_buffer, fwrite_null, fwrite_sha1_file all return the
multiplication of the size and number of items they are being given.
Practically speaking, it doesn't matter, because in all contexts where
those functions are used, size is 1.
But those functions being similar to fread and fwrite (the curl API is
designed around being able to use fread and fwrite directly), it might
be preferable to make them behave like fread and fwrite, which, from
the fread/fwrite manual page, is:
On success, fread() and fwrite() return the number of items read
or written. This number equals the number of bytes transferred
only when size is 1. If an error occurs, or the end of the file
is reached, the return value is a short item count (or zero).
Signed-off-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
With this change, the `index_state` struct becomes the new home for the
flag that says whether the fsmonitor hook has been run, i.e. it is now
per-index.
It also gets re-set when the index is discarded, fixing the bug
demonstrated by the "test_expect_failure" test added in the preceding
commit. In that case fsmonitor-enabled Git would miss updates under
certain circumstances, see that preceding commit for details.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This one is tricky.
When `core.fsmonitor` is set, a `refresh_index()` will not perform a
full scan of files that might be modified, but will query the fsmonitor
and refresh only the ones that have been actually touched.
Due to implementation details, the fsmonitor is queried in
`refresh_cache_ent()`, but of course it only has to be queried once, so
we set a flag when we did that. But when the index was discarded, we did
not re-set that flag.
So far, this is only covered by our test suite when running with
GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all, and only due to the way the
built-in stash interacts with the recursive merge machinery.
Let's introduce a straight-forward regression test for this.
We simply extend the "read & discard index" loop in `test-tool
read-cache` to optionally refresh the index, report on a given file's
status, and then modify that file. Due to the bug described above, only
the first refresh will actually query the fsmonitor; subsequent loop
iterations will not.
This problem was reported by Ævar Arnfjörð Bjarmason.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As noted in preceding commits setting GIT_TEST_INSTALLED has never
been supported or documented, and as noted in an earlier t/perf/README
change to the extent that it's been documented nobody's notices that
the example hasn't worked since 3c8f12c96c ("test-lib: reorder and
include GIT-BUILD-OPTIONS a lot earlier", 2012-06-24).
We could directly support GIT_TEST_INSTALLED for invocations without
the "run" script, such as:
GIT_TEST_INSTALLED=../../ ./p0000-perf-lib-sanity.sh
GIT_TEST_INSTALLED=/home/avar/g/git ./p0000-perf-lib-sanity.sh
But while not having this "error" will "work", it won't write the the
resulting "test-results/*" files to the right place, and thus a
subsequent call to aggregate.perl won't work as expected.
Let's just tell the user that they need to use the "run" script,
which'll correctly deal with this and set the right
PERF_RESULTS_PREFIX.
If someone's in desperate need of bypassing "run" for whatever reason
they can trivially do so by setting "PERF_SET_GIT_TEST_INSTALLED", but
not we won't have people who expect GIT_TEST_INSTALLED to just work
wondering why their aggregation doesn't work, even though they're
running the right "git".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Change the output file names in test-results/ to be
"test-results/bindir_<munged dir>" rather than just
"test-results/<munged dir>".
This is for consistency with the "build_" directories we have for
built revisions, i.e. "test-results/build_<SHA-1>".
There's no user-visible functional changes here, it just makes it
easier to see at a glance what "test-results" files are of what "type"
as they're all explicitly grouped together now, and to grep this code
to find both the run_dirs_helper() implementation and its
corresponding aggregate.perl code.
Note that we already guarantee that the rest of the
PERF_RESULTS_PREFIX is an absolute path, and since it'll start with
e.g. "/" which we munge to "_" we'll up with a readable string like
"bindir_home_avar[...]".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Follow-up my preceding change which fixed the immediate "./run
<revisions>" regression in 0baf78e7bc ("perf-lib.sh: rely on
test-lib.sh for --tee handling", 2019-03-15) and entirely get rid of
GIT_TEST_INSTALLED from perf-lib.sh (and aggregate.perl).
As noted in that change the dance we're doing with GIT_TEST_INSTALLED
perf-lib.sh isn't necessary, but there I was doing the most minimal
set of changes to quickly fix a regression.
But it's much simpler to never deal with the "GIT_TEST_INSTALLED" we
were setting in perf-lib.sh at all. Instead the run_dirs_helper() sets
the previously inferred $PERF_RESULTS_PREFIX directly.
Setting this at the callsite that's already best positioned to
exhaustively know about all the different cases we need to handle
where PERF_RESULTS_PREFIX isn't what we want already (the empty
string) makes the most sense. In one-off cases like:
./run ./p0000-perf-lib-sanity.sh
./p0000-perf-lib-sanity.sh
We'll just do the right thing because PERF_RESULTS_PREFIX will be
empty, and test-lib.sh takes care of finding where our git is.
Any refactoring of this code needs to change both the shell code and
the Perl code in aggregate.perl, because when running e.g.:
./run ../../ -- <test>
The "../../" path to a relative bindir needs to be munged to a
filename containing the results, and critically aggregate.perl does
not get passed the path to those aggregations, just "../..".
Let's fix cases where aggregate.perl would print e.g. ".." in its
report output for this, and "git" for "/home/avar/g/git", i.e. it
would always pick the last element. Now'll always print the full path
instead.
This also makes the code sturdier, e.g. you can feed "../.." to
"./run" and then an absolute path to the aggregate.perl script, as
long as the absolute path and "../.." resolved to the same directory
printing the aggregation will work.
Also simplify the "[_*]" on the RHS of "tr -c", we're trimming
everything to "_", so we don't need that.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Fix a really bad regression in 0baf78e7bc ("perf-lib.sh: rely on
test-lib.sh for --tee handling", 2019-03-15). Since that change all
runs of different <revisions> of git have used the git found in the
user's $PATH, e.g. /usr/bin/git instead of the <revision> we just
built and wanted to performance test.
The problem starts with GIT_TEST_INSTALLED not working like our
non-perf tests with the "run" script. I.e. you can't run performance
tests against a given installed git. Instead we expect to use it
ourselves to point GIT_TEST_INSTALLED to the <revision> we just built.
However, we had been relying on '$(cd "$GIT_TEST_INSTALLED" && pwd)'
to resolve that relative $GIT_TEST_INSTALLED to an absolute
path *before* test-lib.sh was loaded, in cases where it was
e.g. "build/<rev>/bin-wrappers" and we wanted "<abs_path>build/...".
This change post-dates another proposed solution by a few days[1], I
didn't notice that version when I initially wrote this. I'm doing the
most minimal thing to solve the regression here, a follow-up change
will move this result prefix selection logic entirely into the "run"
script.
This makes e.g. these cases all work:
./run . $PWD/../../ origin/master origin/next HEAD -- <tests>
As well as just a plain one-off:
./run <tests>
And, since we're passing down the new GIT_PERF_DIR_MYDIR_REL we make
sure the bug relating to aggregate.perl not finding our files as
described in 0baf78e7bc doesn't happen again.
What *doesn't* work is setting GIT_TEST_INSTALLED to a relative path,
this will subtly fail in test-lib.sh. This has always been the case
even before 0baf78e7bc, and as documented in t/README the
GIT_TEST_INSTALLED variable should be set to an absolute path (needs
to be set "to the bindir", which is always absolute), and the "perf"
framework expects to munge it itself.
Perhaps that should be dealt with in the future to allow manually
setting GIT_TEST_INSTALLED, but as a preceding commit showed the user
can just use the "run" script, which'll also pick the right output
directory for the test results as expected by aggregate.perl.
1. https://public-inbox.org/git/20190502222409.GA15631@sigill.intra.peff.net/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Remove the setting of the "environment" from the --codespeed output. I
don't think this is useful, and it helps with a later refactoring
where we GIT_TEST_INSTALLED stop munging/reading GIT_TEST_INSTALLED in
the perf tests in so many places.
This was added in 05eb1c37ed ("perf/aggregate: implement codespeed
JSON output", 2018-01-05), but since the "run" scripts uses
"GIT_TEST_INSTALLED" internally this was only ever useful for one-off
runs of a single revision as all the "environment" values would be
ones for whatever directory the "run" script ran last.
Let's instead fall back on the "uname -r" case, which is the sort of
thing the environment should be set to, not something that duplicates
other parts of the codpseed output. For setting the "environment" to
something custom the perf.repoName variable can be used. See
19cf57a92e ("perf/run: read GIT_PERF_REPO_NAME from perf.repoName",
2018-01-05).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Since 3c8f12c96c ("test-lib: reorder and include GIT-BUILD-OPTIONS a
lot earlier", 2012-06-24) the suggested advice of overriding
GIT_BUILD_DIR has not worked. We've printed a hard error like this
given e.g. GIT_BUILD_DIR=/home/avar/g/git:
/bin-wrappers/git is not executable; using GIT_EXEC_PATH
error: You haven't built things yet, have you?
Let's just suggest that the user run other gits via the "run"
script. That'll do the right thing for setting the path to the other
git, and running the "aggregate.perl" scripts afterwards will work.
As an aside, if setting GIT_BUILD_DIR had still worked, then the
MODERN_GIT feature/fix added in 1a0962dee5 ("t/perf: fix regression in
testing older versions of git", 2016-06-22) would have broke.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
The revision ':README' is mentioned as an example for '<rev>:<path>'
but the explanation forwards to the ':<n>:<path>' syntax. At the same
time ':<n>:<path>' did not mark the '<n>:' as optional.
Signed-off-by: Andreas Heiduk <asheiduk@gmail.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In revisions.txt, the '<rev>^' form is mentioned but the '<rev>~' form
is missing. Although both forms are essentially equivalent (they each
get the first parent of the specified revision), we should mention the
latter for completeness. Make this change.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In revisions.txt, an optional rev argument was not distinguised.
Instead, a user had to continue and read the description in order to
learn that the argument was optional.
Since the [] notation for an optional argument is common-knowledge in
the Git documentation, mark optional arguments with [] so that it's more
obvious for the reader.
Helped-by: Andreas Heiduk <asheiduk@gmail.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In revisions.txt, there were some instances of a rev argument being
written as "rev". However, since they didn't mean the string literal,
write "<rev>", instead.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
'flags' parameter was replaced by 'raw' in commit:
76141e2e62 ("format_note(): simplify API", 2012-10-17)
Signed-off-by: Chris Mayo <aklhfex@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To support IPv6, Git provided fall back functions for Windows versions
that did not support IPv6. However, as Git dropped support for Windows
XP and prior, those functions are not needed anymore.
Remove those fallbacks by reverting fe3b2b7b82 (Enable support for
IPv6 on MinGW, 2009-11-24) and using the functions directly (without
'ipv6_' prefix).
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
On Windows, UNC paths are a very convenient way to share data, and
alternates are all about sharing data.
We fixed a bug where alternates specifying UNC paths were not handled
properly, and it is high time that we add a regression test to ensure
that this bug is not reintroduced.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When using cvsnt + msys + git, it seems like the output of cvs status
had \r\n in it, and caused the command to fail.
This fixes that.
Signed-off-by: Dustin Spicuzza <dustin@virtualroadside.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since e7e9f5e7a1 (travis-ci: enable Git SVN tests t91xx on Linux,
2016-05-19) some of our Travis CI build jobs install the 'git-svn'
package, because it was a convenient way to install its dependencies,
which are necessary to run our 'git-svn' tests (we don't actually need
the 'git-svn' package itself). However, from those dependencies,
namely the 'libsvn-perl', 'libyaml-perl', and 'libterm-readkey-perl'
packages, only 'libsvn-perl' is necessary to run those tests, the
others arent, not even to fulfill some prereqs.
So update 'ci/install-dependencies.sh' to install only 'libsvn-perl'
instead of 'git-svn' and its additional dependencies.
Note that this change has more important implications than merely not
installing three unnecessary packages, as it keeps our builds working
with Travis CI's Xenial images. In our '.travis.yml' we never
explicitly specified which Linux image we want to use to run our Linux
build jobs, and so far they have been run on the default Ubuntu 14.04
Trusty image. However, 14.04 just reached its EOL, and Travis CI has
already began the transition to use 16.04 Xenial as the default Linux
build environment [1]. Alas, our Linux Clang and GCC build jobs can't
simply 'apt-get install git-svn' in the current Xenial images [2],
like they did in the Trusty images, and, consequently, fail.
Installing only 'libsvn-perl' avoids this issue, while the 'git svn'
tests are still run as they should.
[1] https://blog.travis-ci.com/2019-04-15-xenial-default-build-environment
[2] 'apt-get install git-svn' in the Xenial image fails with:
The following packages have unmet dependencies:
git-svn : Depends: git (< 1:2.7.4-.)
E: Unable to correct problems, you have held broken packages.
The reason is that both the Trusty and Xenial images contain the
'git' package installed from 'ppa:git-core/ppa', so it's
considerably newer than the 'git' package in the corresponding
standard Ubuntu package repositories. The difference is that the
Trusty image still contains these third-party apt repositories, so
the 'git-svn' package was installed from the same PPA, and its
version matched the version of the already installed 'git'
package. In the Xenial image, however, these third-party
apt-repositories are removed (to reduce the risk of unrelated
interference and faster 'apt-get update') [3], and the version of
the 'git-svn' package coming from the standard Ubuntu package
repositories doesn't match the much more recent version of the
'git' package installed from the PPA, resulting in this dependecy
error.
Adding back the 'ppa:git-core/ppa' package repository would solve
this dependency issue as well, but since the troublesome package
happens to be unnecessary, not installing it in the first place is
better.
[3] https://docs.travis-ci.com/user/reference/xenial/#third-party-apt-repositories-removed
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The exit code of the upstream in a pipe is ignored thus we should avoid
using it. By writing out the output of the git command to a file, we can
test the exit codes of both the commands.
Signed-off-by: Boxuan Li <liboxuan@connect.hku.hk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Free the commit graph when verify_commit_graph_lite() reports an error.
Credit to OSS-Fuzz for finding this leak.
Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The multi-pack-index allows searching for objects across multiple
packs using one object list. The original design gains many of
these performance benefits by keeping the packs in the
multi-pack-index out of the packed_git list.
Unfortunately, this has one major drawback. If the multi-pack-index
covers thousands of packs, and a command loads many of those packs,
then we can hit the limit for open file descriptors. The
close_one_pack() method is used to limit this resource, but it
only looks at the packed_git list, and uses an LRU cache to prevent
thrashing.
Instead of complicating this close_one_pack() logic to include
direct references to the multi-pack-index, simply add the packs
opened by the multi-pack-index to the packed_git list. This
immediately solves the file-descriptor limit problem, but requires
some extra steps to avoid performance issues or other problems:
1. Create a multi_pack_index bit in the packed_git struct that is
one if and only if the pack was loaded from a multi-pack-index.
2. Skip packs with the multi_pack_index bit when doing object
lookups and abbreviations. These algorithms already check the
multi-pack-index before the packed_git struct. This has a very
small performance hit, as we need to walk more packed_git
structs. This is acceptable, since these operations run binary
search on the other packs, so this walk-and-ignore logic is
very fast by comparison.
3. When closing a multi-pack-index file, do not close its packs,
as those packs will be closed using close_all_packs(). In some
cases, such as 'git repack', we run 'close_midx()' without also
closing the packs, so we need to un-set the multi_pack_index bit
in those packs. This is necessary, and caught by running
t6501-freshen-objects.sh with GIT_TEST_MULTI_PACK_INDEX=1.
To manually test this change, I inserted trace2 logging into
close_pack_fd() and set pack_max_fds to 10, then ran 'git rev-list
--all --objects' on a copy of the Git repo with 300+ pack-files and
a multi-pack-index. The logs verified the packs are closed as
we read them beyond the file descriptor limit.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Much of the multi-pack-index code focuses on the multi_pack_index
struct, and so we only pass a pointer to the current one. However,
we will insert a dependency on the packed_git linked list in a
future change, so we will need a repository reference. Inserting
these parameters is a significant enough change to split out.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we ran something like
$ git checkout -b test master...
it would fail with the message
fatal: Not a valid object name: 'master...'.
This was caused by the call to `create_branch` where `start_name` is
expected to be a valid rev. However, git-checkout allows the branch to
be a valid _merge base_ rev (i.e. with a "...") so it was possible for
an invalid rev to be passed in.
Make `create_branch` accept a merge base rev so that this case does not
error out.
As a side-effect, teach git-branch how to handle merge base revs as
well.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Before, in t2018, if do_checkout failed to create `branch2`, the next
test-case would run `git branch -D branch2` but then fail because it was
expecting `branch2` to exist, even though it doesn't. As a result, an
early failure could cause a cascading failure of tests.
Make test-case responsible for cleaning up their own branches so that
future tests can start with a sane environment.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the option parsing machinery so that e.g. "clone --recurs ..."
doesn't error out because "clone" understands both "--recursive" and
"--recurse-submodules" to mean the same thing.
Initially "clone" just understood --recursive until the
--recurses-submodules alias was added in ccdd3da652 ("clone: Add the
--recurse-submodules option as alias for --recursive",
2010-11-04). Since bb62e0a99f ("clone: teach --recurse-submodules to
optionally take a pathspec", 2017-03-17) the longer form has been
promoted to the default.
But due to the way the options parsing machinery works this resulted
in the rather absurd situation of:
$ git clone --recurs [...]
error: ambiguous option: recurs (could be --recursive or --recurse-submodules)
Add OPT_ALIAS() to express this link between two or more options and use
it in git-clone. Multiple aliases of an option could be written as
OPT_ALIAS(0, "alias1", "original-name"),
OPT_ALIAS(0, "alias2", "original-name"),
...
The current implementation is not exactly optimal in this case. But we
can optimize it when it becomes a problem. So far we don't even have two
aliases of any option.
A big chunk of code is actually from Junio C Hamano.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In our "make coccicheck" rule, we currently feed each source file to its
own individual invocation of spatch. This has a few downsides:
- it repeats any overhead spatch has for starting up and reading the
patch file
- any included header files may get processed from multiple
invocations. This is slow (we see the same header files multiple
times) and may produce a resulting patch with repeated hunks (which
cannot be applied without further cleanup)
Ideally we'd just invoke a single instance of spatch per rule-file and
feed it all source files. But spatch can be rather memory hungry when
run in this way. I measured the peak RSS going from ~90MB for a single
file to ~1900MB for all files. Multiplied by multiple rule files being
processed at the same time (for "make -j"), this can make things slower
or even cause them to fail (e.g., this is reported to happen on our
Travis builds).
Instead, let's provide a tunable knob. We'll leave the default at "1",
but it can be cranked up to "999" for maximum CPU/memory tradeoff, or
people can find points in between that serve their particular machines.
Here are a few numbers running a single rule via:
SIZES='1 4 16 999'
RULE=contrib/coccinelle/object_id.cocci
for i in $SIZES; do
make clean
/usr/bin/time -o $i.out --format='%e | %U | %S | %M' \
make $RULE.patch SPATCH_BATCH_SIZE=$i
done
for i in $SIZES; do
printf '%4d | %s\n' $i "$(cat $i.out)"
done
which yields:
1 | 97.73 | 93.38 | 4.33 | 100128
4 | 52.80 | 51.14 | 1.69 | 135204
16 | 35.82 | 35.09 | 0.76 | 284124
999 | 23.30 | 23.13 | 0.20 | 1903852
The implementation is done with xargs, which should be widely available;
it's in POSIX, we rely on it already in the test suite. And "coccicheck"
is really a developer-only tool anyway, so it's not a big deal if
obscure systems can't run it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In previous patches, extern was mechanically removed from function
declarations without care to formatting, causing parameter lists to be
misaligned. Manually format changed sections such that the parameter
lists should be realigned.
Viewing this patch with 'git diff -w' should produce no output.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There has been a push to remove extern from function declarations.
Finish the job by removing all instances of "extern" for function
declarations in headers using sed.
This was done by running the following on my system with sed 4.2.2:
$ git ls-files \*.{c,h} |
grep -v ^compat/ |
xargs sed -i'' -e 's/^\(\s*\)extern \([^(]*([^*]\)/\1\2/'
Files under `compat/` are intentionally excluded as some are directly
copied from external sources and we should avoid churning them as much
as possible.
Then, leftover instances of extern were found by running
$ git grep -w -C3 extern \*.{c,h}
and manually checking the output. No other instances were found.
Note that the regex used specifically excludes function variables which
_should_ be left as extern.
Not the most elegant way to do it but it gets the job done.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There has been a push to remove extern from function declarations.
Remove some instances of "extern" for function declarations which are
caught by Coccinelle. Note that Coccinelle has some difficulty with
processing functions with `__attribute__` or varargs so some `extern`
declarations are left behind to be dealt with in a future patch.
This was the Coccinelle patch used:
@@
type T;
identifier f;
@@
- extern
T f(...);
and it was run with:
$ git ls-files \*.{c,h} |
grep -v ^compat/ |
xargs spatch --sp-file contrib/coccinelle/noextern.cocci --in-place
Files under `compat/` are intentionally excluded as some are directly
copied from external sources and we should avoid churning them as much
as possible.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
