Since 04519d7201 (rebase: validate -C<n> and --whitespace=<mode>
parameters early, 2018-11-14), the built-in rebase validates the -C and
--whitespace arguments early. As this commit also introduced a
regression test for this, and as a later commit introduced the
GIT_TEST_REBASE_USE_BUILTIN mode to run tests, we now have a
"regression" in the scripted version of `git rebase` on our hands.
Backport the validation to fix this.
Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Amend the progress output added in 7b0f229222 ("commit-graph write:
add progress output", 2018-09-17) so that the total numbers it reports
aren't higher than the total number of commits anymore. See [1] for a
bug report pointing that out.
When I added this I wasn't intending to provide an accurate count, but
just have some progress output to show the user the command wasn't
hanging[2]. But since we are showing numbers, let's make them
accurate. The progress descriptions were suggested by Derrick Stolee
in [3].
As noted in [2] we are unlikely to show anything except the "Expanding
reachable..." message even on fairly large repositories such as
linux.git. On a test repository I have with north of 7 million commits
all of these are displayed. Two of them don't show up for long, but as
noted in [5] future-proofing this for if the loops become more
expensive in the future makes sense.
1. https://public-inbox.org/git/20181010203738.GE23446@szeder.dev/
2. https://public-inbox.org/git/87pnwhea8y.fsf@evledraar.gmail.com/
3. https://public-inbox.org/git/f7a0cbee-863c-61d3-4959-5cec8b43c705@gmail.com/
4. https://public-inbox.org/git/20181015160545.GG19800@szeder.dev/
5. https://public-inbox.org/git/87murle8da.fsf@evledraar.gmail.com/
Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Helped-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The 'test_cmp_rev' helper is merely a wrapper around 'test_cmp'
checking the output of two 'git rev-parse' commands, which means that
its output on failure is not particularly informative, as it's
basically two OIDs with a bit of extra clutter of the diff header, but
without any indication of which two revisions have caused the failure:
--- expect.rev 2018-11-17 14:02:11.569747033 +0000
+++ actual.rev 2018-11-17 14:02:11.569747033 +0000
@@ -1 +1 @@
-d79ce1670bdcb76e6d1da2ae095e890ccb326ae9
+139b20d8e6c5b496de61f033f642d0e3dbff528d
It also pollutes the test repo with these two intermediate files,
though that doesn't seem to cause any complications in our current
tests (meaning that I couldn't find any tests that have to work around
the presence of these files by explicitly removing or ignoring them).
Enhance 'test_cmp_rev' to provide a more useful output on failure with
less clutter:
error: two revisions point to different objects:
'HEAD^': d79ce1670bdcb76e6d1da2ae095e890ccb326ae9
'extra': 139b20d8e6c5b496de61f033f642d0e3dbff528d
Doing so is more convenient when storing the OIDs outputted by 'git
rev-parse' in a local variable each, which, as a bonus, won't pollute
the repository with intermediate files.
While at it, also ensure that 'test_cmp_rev' is invoked with the right
number of parameters, namely two.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some of the functions in our test library check that they were invoked
properly with conditions like this:
test "$#" = 2 ||
error "bug in the test script: not 2 parameters to test-expect-success"
If this particular condition is triggered, then 'error' will abort the
whole test script with a bold red error message [1] right away.
However, under certain circumstances the test script will be aborted
completely silently, namely if:
- a similar condition in a test helper function like
'test_line_count' is triggered,
- which is invoked from the test script's "main" shell [2],
- and the test script is run manually (i.e. './t1234-foo.sh' as
opposed to 'make t1234-foo.sh' or 'make test') [3]
- and without the '--verbose' option,
because the error message is printed from within 'test_eval_', where
standard output is redirected either to /dev/null or to a log file.
The only indication that something is wrong is that not all tests in
the script are executed and at the end of the test script's output
there is no "# passed all N tests" message, which are subtle and can
easily go unnoticed, as I had to experience myself.
Send these "bug in the test script" error messages directly to the
test scripts standard error and thus to the terminal, so those bugs
will be much harder to overlook. Instead of updating all ~20 such
'error' calls with a redirection, let's add a BUG() function to
'test-lib.sh', wrapping an 'error' call with the proper redirection
and also including the common prefix of those error messages, and
convert all those call sites [4] to use this new BUG() function
instead.
[1] That particular error message from 'test_expect_success' is
printed in color only when running with or without '--verbose';
with '--tee' or '--verbose-log' the error is printed without
color, but it is printed to the terminal nonetheless.
[2] If such a condition is triggered in a subshell of a test, then
'error' won't be able to abort the whole test script, but only the
subshell, which in turn causes the test to fail in the usual way,
indicating loudly and clearly that something is wrong.
[3] Well, 'error' aborts the test script the same way when run
manually or by 'make' or 'prove', but both 'make' and 'prove' pay
attention to the test script's exit status, and even a silently
aborted test script would then trigger those tools' usual
noticable error messages.
[4] Strictly speaking, not all those 'error' calls need that
redirection to send their output to the terminal, see e.g.
'test_expect_success' in the opening example, but I think it's
better to be consistent.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This also removes an implicit conversion from size_t (unsigned) to int (signed).
_stricmp as well as _strnicmp are both available since VS2012.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A coding convention around the Coccinelle semantic patches to have
two classes to ease code migration process has been proposed and
its support has been added to the Makefile.
* sb/cocci-pending:
coccicheck: introduce 'pending' semantic patches
Update the "test installed Git" mode of our test suite to work better.
* js/test-git-installed:
tests: explicitly use `git.exe` on Windows
tests: do not require Git to be built when testing an installed Git
t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set
tests: respect GIT_TEST_INSTALLED when initializing repositories
tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/
Code preparation to replace ulong vars with size_t vars where
appropriate.
* tb/print-size-t-with-uintmax-format:
Upcast size_t variables to uintmax_t when printing
The xcurl_off_t() helper function is used to cast size_t to
curl_off_t, but some compilers gave warnings against the code to
ensure the casting is done without wraparound, when size_t is
narrower than curl_off_t. This warning has been squelched.
* tb/xcurl-off-t:
remote-curl.c: xcurl_off_t is not portable (on 32 bit platfoms)
"git format-patch --stat=<width>" can be used to specify the width
used by the diffstat (shown in the cover letter).
* nd/format-patch-cover-letter-stat-width:
format-patch: respect --stat in cover letter's diffstat
"git push" used to check ambiguities between object-names and
refnames while processing the list of refs' old and new values,
which was unnecessary (as it knew that it is feeding raw object
names). This has been optimized out.
* ds/push-squelch-ambig-warning:
pack-objects: ignore ambiguous object warnings
Our testing framework uses a special i18n "poisoned localization"
feature to find messages that ought to stay constant but are
incorrectly marked to be translated. This feature has been made
into a runtime option (it used to be a compile-time option).
* ab/dynamic-gettext-poison:
Makefile: ease dynamic-gettext-poison transition
i18n: make GETTEXT_POISON a runtime option
This lets us use :(attr) with "git grep <tree-ish>" or "git log".
:(attr) requires another round of checking before we can declare that
a path is matched. This is done after path matching since we have lots
of optimization to take a shortcut when things don't match.
Note that if :(attr) is present, we can't return
all_entries_interesting / all_entries_not_interesting anymore because
we can't be certain about that. Not until match_pathspec_attrs() can
tell us "yes all these paths satisfy :(attr)".
Second note. Even though we walk a specific tree, we use attributes
from _worktree_ (or falling back to the index), not from .gitattributes
files on that tree. This by itself is not necessarily wrong, but the
user just have to be aware of this.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function will be reused for matching attributes in pathspec when
walking trees (currently it's used for matching pathspec when walking
a list). pathspec.c would be a more neutral place for this.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"extern" on functions is not required and the trend has been removing
it from header files.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In order to support :(attr) when matching pathspec on a tree,
tree_entry_interesting() needs to take an index (because
git_check_attr() needs it). This is the preparation step for it. This
also makes it clearer what index we fall back to when looking up
attributes during an unpack-trees operation: the source index.
This also fixes revs->pruning.repo initialization that should have
been done in 2abf350385 (revision.c: remove implicit dependency on
the_index - 2018-09-21). Without it, skip_uninteresting() will
dereference a NULL pointer through this call chain
get_revision(revs)
get_revision_internal
get_revision_1
try_to_simplify_commit
rev_compare_tree
diff_tree_oid(..., &revs->pruning)
ll_diff_tree_oid
diff_tree_paths
ll_diff_tree
skip_uninteresting
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These functions call tree_entry_interesting() which will soon require
a 'struct index_state *' to be passed in. Instead of just changing the
function signature to take an index, update to take a repo instead
because these functions do need object database access.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make the split index write out its .git/sharedindex_* files with the
same permissions as .git/index. This only changes the behavior when
core.sharedRepository isn't set, i.e. the user's umask settings will
be respected.
This hasn't been the case ever since the split index was originally
implemented in c18b80a0e8 ("update-index: new options to
enable/disable split index mode", 2014-06-13). A mkstemp()-like
function has always been used to create it. First mkstemp() itself,
and then later our own mkstemp()-like in
f6ecc62dbf ("write_shared_index(): use tempfile module", 2015-08-10)
A related bug was fixed in df801f3f9f ("read-cache: use shared perms
when writing shared index", 2017-06-25). Since then the split index
has respected core.sharedRepository.
However, using that setting should not be required simply to make git
obey the user's umask setting. It's intended for the use-case of
overriding whatever that umask is set to. This fixes cases where the
user has e.g. set his umask to 022 on a shared server in anticipation
of other user's needing to run "status", "log" etc. in his repository.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "git stash" command insists on having a usable user identity to
the same degree as the "git commit-tree" and "git commit" commands
do, because it uses the same codepath that creates commit objects
as these commands.
It is not strictly necesary to do so. Check if we will barf before
creating commit objects and then supply fake identity to please the
machinery that creates commits.
Add test to document that stash executes correctly both with and
without valid ident.
This is not that much of usability improvement, as the users who run
"git stash" would eventually want to record their changes that are
temporarily stored in the stashes in a more permanent history by
committing, and they must do "git config user.{name,email}" at that
point anyway, so arguably this change is only delaying a step that
is necessary to work in the repository.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When "git bundle" aborts due to an empty commit ranges
(i.e. resulting in an empty pack), it left a file descriptor to an
lockfile open, which resulted in leftover lockfile on Windows where
you cannot remove a file with an open file descriptor. This has
been corrected.
* jk/close-duped-fd-before-unlock-for-bundle:
bundle: dup() output descriptor closer to point-of-use
The recently merged "rebase in C" has an escape hatch to use the
scripted version when necessary, but it hasn't been documented,
which has been corrected.
* ab/rebase-in-c-escape-hatch:
tests: add a special setup where rebase.useBuiltin is off
rebase doc: document rebase.useBuiltin
The way "git rebase" parses and forwards the command line options
meant for underlying "git am" has been revamped, which fixed for
options with parameters that were not passed correctly.
* js/rebase-am-options:
rebase: validate -C<n> and --whitespace=<mode> parameters early
rebase: really just passthru the `git am` options
"git ls-remote --sort=<thing>" can feed an object that is not yet
available into the comparison machinery and segfault, which has
been corrected to check such a request upfront and reject it.
* sg/ref-filter-wo-repository:
ref-filter: don't look for objects when outside of a repository
The build procedure to link for fuzzing test has been made
customizable with a new Makefile variable.
* js/fuzz-cxxflags:
Makefile: use FUZZ_CXXFLAGS for linking fuzzers
A sanity check for start-up sequence has been added in the config
API codepath.
* js/config-sequence:
config: report a bug if git_dir exists without commondir
Bugfix for the recently graduated "git rebase --rebase-merges".
* js/rebase-r-and-merge-head:
status: rebase and merge can be in progress at the same time
built-in rebase --skip/--abort: clean up stale .git/<name> files
rebase -i: include MERGE_HEAD into files to clean up
rebase -r: do not write MERGE_HEAD unless needed
rebase -r: demonstrate bug with conflicting merges
When editing a patch in a "git add -i" session, a hunk could be
made to no-op. The "git apply" program used to reject a patch with
such a no-op hunk to catch user mistakes, but it is now updated to
explicitly allow a no-op hunk in an edited patch.
* js/apply-recount-allow-noop:
apply --recount: allow "no-op hunks"
"rev-parse --exclude=<pattern> --branches=<pattern>" etc. did not
quite work, which has been corrected.
* ra/rev-parse-exclude-glob:
refs: fix some exclude patterns being ignored
refs: show --exclude failure with --branches/tags/remotes=glob
Code clean-up with correction to make the reimplemented "git
rebase" a more faithful rewrite of the original, which also regains
performance.
* js/builtin-rebase-perf-fix:
built-in rebase: reinstate `checkout -q` behavior where appropriate
rebase: prepare reset_head() for more flags
rebase: consolidate clean-up code before leaving reset_head()
"git rebase --autostash" did not correctly re-attach the HEAD at times.
* js/rebase-autostash-detach-fix:
built-in rebase --autostash: leave the current branch alone if possible
built-in rebase: demonstrate regression with --autostash
The "--no-patch" option, which can be used to get a high-level
overview without the actual line-by-line patch difference shown, of
the "range-diff" command was earlier broken, which has been
corrected.
* ab/range-diff-no-patch:
range-diff: make diff option behavior (e.g. --stat) consistent
range-diff: fix regression in passing along diff options
range-diff doc: add a section about output stability
"git merge" and "git pull" that merges into an unborn branch used
to completely ignore "--verify-signatures", which has been
corrected.
* jk/verify-sig-merge-into-void:
pull: handle --verify-signatures for unborn branch
merge: handle --verify-signatures for unborn branch
merge: extract verify_merge_signature() helper
Various functions have been audited for "-Wunused-parameter" warnings
and bugs in them got fixed.
* jk/unused-parameter-fixes:
midx: double-check large object write loop
assert NOARG/NONEG behavior of parse-options callbacks
parse-options: drop OPT_DATE()
apply: return -1 from option callback instead of calling exit(1)
cat-file: report an error on multiple --batch options
tag: mark "--message" option with NONEG
show-branch: mark --reflog option as NONEG
format-patch: mark "--no-numbered" option with NONEG
status: mark --find-renames option with NONEG
cat-file: mark batch options with NONEG
pack-objects: mark index-version option as NONEG
ls-files: mark exclude options as NONEG
am: handle --no-patch-format option
apply: mark include/exclude options as NONEG
