When collecting the list of objects to pack in get_object_list(), we
pass our rev_info struct around to some functions that don't need it.
This is due to 03a9683d22 (Simplify is_kept_pack(), 2009-02-28), where
the kept-pack handling was moved out of the revision machinery.
Let's drop these unused parameters.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When this function was extracted in 0041bf6544 (name-rev: refactor logic
to see if a new candidate is a better name, 2017-03-29), it ended up
getting more arguments than it needs.
It's possible we may later use these values to evaluate the name, but
since it's a static function with a single caller, it will be easy to
add them back then.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The mktree_line() function does not actually look at the "len" parameter
it is passed, and assumes the buffer it receives is NUL-terminated.
Since the caller always passes a strbuf, this will be true. Let's drop
the useless parameter.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The v2_fix_up_changed() function doesn't actually need to see the
wt_status struct. It's possible that could change in the future, but
this is a static-local function with one caller. It would be easy to
read-add it back then. Let's drop the unused parameter.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The load_cache_entries_threaded() function takes a src_offset parameter
that it doesn't use. This has been there since its inception in
77ff1127a4 (read-cache: load cache entries on worker threads,
2018-10-10).
Digging on the mailing list, that parameter was part of an earlier
iteration of the series[1], but became unnecessary when the code
switched to using the IEOT extension.
[1] https://public-inbox.org/git/20180906210227.54368-5-benpeart@microsoft.com/
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ever since the inception of this function in e6baf4a1ae (clone: clone
from a repository with relative alternates, 2011-08-22), the "dest"
parameter has been unused. Instead, we use add_to_alternates_file(),
which relies on git_pathdup() to find the right file. That in turn works
because we will have initialized and entered the destination repo by
this point.
It's a bit subtle, but this is how it has always worked. And if our
assumptions change, the test in t5601 from e6baf4a1ae should let us
know.
In the meantime, let's drop this unused and confusing parameter from
copy_alternates().
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We stopped using the "prefix" parameter of
relocate_single_git_dir_into_superproject() and its callers in
202275b96b (submodule.c: get_super_prefix_or_empty, 2017-03-14), where
we switched to using the environment global directly.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If a builtin uses RUN_SETUP to request that git.c enter the repository
directory, we'll get passed in a "prefix" variable with the path to the
original directory. It's important to pass this to parse_options(),
since we may use it to fix up relative OPT_FILENAME() options. Some
builtins don't bother; let's make sure we do so consistently.
There may not be any particular bugs fixed here; OPT_FILENAME is
actually pretty rare, and none of these commands use it directly.
However, this does future-proof us against somebody adding an option
that uses it and creating a subtle bug that only shows up when you're in
a subdirectory of the repository.
In some cases, like hash-object and upload-pack, we don't specify
RUN_SETUP, so we know the prefix will always be empty. It's still worth
passing the variable along to keep the idiom consistent across all
builtins (and of course it protects us if they ever _did_ switch to
using RUN_SETUP).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "prefix" variable passed by git.c into the builtin cmd_read_tree()
and cmd_write_tree() functions is named "unused_prefix". But we do in
fact pass it to parse_options(), which may use the prefix to adjust any
filename options. Let's get rid of this confusing name.
However, we can't just call it "prefix". The reason these variables were
renamed in the first place is that they shadowed local variables named
"prefix", because these commands both take a "--prefix" option.
So let's rename the parameters, but try to reduce further confusion:
1. In both cases we'll call them "cmd_prefix" to mark that they're
part of the cmd_* interface.
2. In cmd_write_tree(), we'll rename the local prefix variable to
"tree_prefix" to make it more clear that we're talking about the
prefix to be used for the tree we're writing.
3. In cmd_read_tree(), the "prefix" local has since migrated into
"struct unpack_trees_options". We'll leave that alone, as the
context within the struct makes its meaning clear (we actually
_could_ just call the parameter "prefix" now, but that invites
confusion in the other direction).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
For thoroughness when checking for one-shot environment variable
assignments at shell function call sites, check-non-portable-shell
stitches together incomplete lines (those ending with backslash). This
allows it to correctly flag such undesirable usage even when the
variable assignment and function call are split across lines, for
example:
FOO=bar \
func
where 'func' is a shell function.
The stitching is accomplished like this:
while (<>) {
chomp;
# stitch together incomplete lines (those ending with "\")
while (s/\\$//) {
$_ .= readline;
chomp;
}
# detect unportable/undesirable shell constructs
...
}
Although this implementation is well supported in reasonably modern Perl
versions (5.10 and later), it fails with older versions (such as Perl
5.8 shipped with ancient Mac OS 10.5). In particular, in older Perl
versions, 'readline' is not connected to the file handle associated with
the "magic" while (<>) {...} construct, so 'readline' throws a
"readline() on unopened filehandle" error. Work around this problem by
dropping readline() and instead incorporating the stitching of
incomplete lines directly into the existing while (<>) {...} loop.
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Enable DEP (Data Execution Prevention) and ASLR (Address Space Layout
Randomization) support. This applies to both 32bit and 64bit builds
and makes it substantially harder to exploit security holes in Git by
offering a much more unpredictable attack surface.
ASLR interferes with GDB's ability to set breakpoints. A similar issue
holds true when compiling with -O2 (in which case single-stepping is
messed up because GDB cannot map the code back to the original source
code properly). Therefore we simply enable ASLR only when an
optimization flag is present in the CFLAGS, using it as an indicator
that the developer does not want to debug in GDB anyway.
Signed-off-by: İsmail Dönmez <ismail@i10z.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is the first step for enabling ASLR (Address Space Layout
Randomization) support. We want to enable ASLR for better protection
against exploiting security holes in Git: it makes it harder to attack
software by making code addresses unpredictable.
The problem fixed by this commit is that `ld.exe` seems to be stripping
relocations which in turn will break ASLR support. We just make sure
it's not stripping the main executable entry.
Signed-off-by: İsmail Dönmez <ismail@i10z.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a trivial bug that's been here since the shared/do_write_index
tracing was added in 42fee7a388 ("trace2:data: add trace2
instrumentation to index read/write", 2019-02-22). We should have
enter/leave points, not two enter/enter points. This resulted in an
"enter" event without a corresponding "leave" event.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Acked-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In `--no-index` mode, we now no longer require a worktree nor a
repository. But some code paths in `difftool` expect those to be
present.
The most notable such code path is the `--dir-diff` one: we use the
existing checkout machinery to copy the files, and that machinery looks
up replacement refs, looks at alternate ODBs, wants to use the worktree
path, etc.
Rather than running into segmentation faults, let's die with an
informative error message.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In eea9c1e78f (tag: advise on nested tags, 2019-04-04), tag was taught
to hint at the user if a nested tag is made. However, this message had a
typo and it said "The object referred to by your new is...", which was
missing a "tag" after "new". Fix this message by adding the "tag".
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use File::Spec->devnull() for output redirection to avoid messages
when Windows version of Perl is first in path. The message 'The
system cannot find the path specified.' is displayed each time git is
run to get colors.
Signed-off-by: Chris. Webster <chris@webstech.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The autoconf generated configure script failed to use the right
gettext() implementations from -libintl by ignoring useless stub
implementations shipped in some C library, which has been
corrected.
* vk/autoconf-gettext:
autoconf: #include <libintl.h> when checking for gettext()
AIX shared the same build issues with other BSDs around fileno(fp),
which has been corrected.
* cc/aix-has-fileno-as-a-macro:
Makefile: use fileno macro work around on AIX
The logic to tell if a Git repository has a working tree protects
"git branch -D" from removing the branch that is currently checked
out by mistake. The implementation of this logic was broken for
repositories with unusual name, which unfortunately is the norm for
submodules these days. This has been fixed.
* jt/submodule-repo-is-with-worktree:
worktree: update is_bare heuristics
Code clean-up.
* jk/untracked-cache-more-fixes:
untracked-cache: simplify parsing by dropping "len"
untracked-cache: simplify parsing by dropping "next"
untracked-cache: be defensive about missing NULs in index
"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>
We weren't flushing the context each time we processed a hunk in the
patch-id generation code in diff.c, but we were doing that when we
generated "stable" patch-ids with the 'patch-id' tool. Let's port that
similar logic over from patch-id.c into diff.c so we can get the same
hash when we're generating patch-ids for 'format-patch --base=' types of
command invocations.
Cc: Xiaolong Ye <xiaolong.ye@intel.com>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
I tried out 'git format-patch --base' with a set of commits that
modifies more than one file. It turns out that the way this command is
implemented it actually uses the unstable version of patch-id instead of
the stable version that's documented. When I tried to modify the
existing test to use 'git patch-id --stable' vs. 'git patch-id
--unstable' I found that it didn't matter, the test still passed.
Let's expand on the test here so it is a little more complicated and
then use that to show that the patch-id generation is actually unstable
vs. stable. Update the documentation as well.
Cc: Xiaolong Ye <xiaolong.ye@intel.com>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
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>
Jeff King
Eric Sunshine
İsmail Dönmez
Ævar Arnfjörð Bjarmason
Johannes Schindelin
Denton Liu
Chris. Webster
Junio C Hamano
Nguyễn Thái Ngọc Duy
Stephen Boyd
Mike Hommey