The "collision detecting" SHA-1 implementation shipped with 2.13
was quite broken on some big-endian platforms and/or platforms that
do not like unaligned fetches. Update to the upstream code which
has already fixed these issues.
The internal implementation of "git grep" has seen some clean-up.
* ab/grep-preparatory-cleanup: (31 commits)
grep: assert that threading is enabled when calling grep_{lock,unlock}
grep: given --threads with NO_PTHREADS=YesPlease, warn
pack-objects: fix buggy warning about threads
pack-objects & index-pack: add test for --threads warning
test-lib: add a PTHREADS prerequisite
grep: move is_fixed() earlier to avoid forward declaration
grep: change internal *pcre* variable & function names to be *pcre1*
grep: change the internal PCRE macro names to be PCRE1
grep: factor test for \0 in grep patterns into a function
grep: remove redundant regflags assignments
grep: catch a missing enum in switch statement
perf: add a comparison test of log --grep regex engines with -F
perf: add a comparison test of log --grep regex engines
perf: add a comparison test of grep regex engines with -F
perf: add a comparison test of grep regex engines
perf: emit progress output when unpacking & building
perf: add a GIT_PERF_MAKE_COMMAND for when *_MAKE_OPTS won't do
grep: add tests to fix blind spots with \0 patterns
grep: prepare for testing binary regexes containing rx metacharacters
grep: add a test helper function for less verbose -f \0 tests
...
The result from "git diff" that compares two blobs, e.g. "git diff
$commit1:$path $commit2:$path", used to be shown with the full
object name as given on the command line, but it is more natural to
use the $path in the output and use it to look up .gitattributes.
* jk/diff-blob:
diff: use blob path for blob/file diffs
diff: use pending "path" if it is available
diff: use the word "path" instead of "name" for blobs
diff: pass whole pending entry in blobinfo
handle_revision_arg: record paths for pending objects
handle_revision_arg: record modes for "a..b" endpoints
t4063: add tests of direct blob diffs
get_sha1_with_context: dynamically allocate oc->path
get_sha1_with_context: always initialize oc->symlink_path
sha1_name: consistently refer to object_context as "oc"
handle_revision_arg: add handle_dotdot() helper
handle_revision_arg: hoist ".." check out of range parsing
handle_revision_arg: stop using "dotdot" as a generic pointer
handle_revision_arg: simplify commit reference lookups
handle_revision_arg: reset "dotdot" consistently
"git clean -d" used to clean directories that has ignored files,
even though the command should not lose ignored ones without "-x".
"git status --ignored" did not list ignored and untracked files
without "-uall". These have been corrected.
* sl/clean-d-ignored-fix:
clean: teach clean -d to preserve ignored paths
dir: expose cmp_name() and check_contains()
dir: hide untracked contents of untracked dirs
dir: recurse into untracked dirs for ignored files
t7061: status --ignored should search untracked dirs
t7300: clean -d should skip dirs with ignored files
"foo\bar\baz" in "git fetch foo\bar\baz", even though there is no
slashes in it, cannot be a nickname for a remote on Windows, as
that is likely to be a pathname on a local filesystem.
* js/bs-is-a-dir-sep-on-windows:
Windows: do not treat a path with backslashes as a remote's nick name
mingw.h: permit arguments with side effects for is_dir_sep
Unless --no-validate is passed, send-email will invoke
$repo->repo_path() in its search for a validate hook regardless of
whether a Git repo is actually present. Teach send-email to first check
for repo existence.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
send-email: Net::SMTP::starttls was introduced in v2.34
We cannot rely on the starttls method being present in Net::SMTP until c274b798e6881a941d941808c6d89966975cb8c8 (Merge branch 'ipv6_ssl' of
https://github.com/noxxi/perl-libnet into noxxi-ipv6_ssl, 2014-06-02),
which set the module version to 2.34.
This version was first shipped as part of perl in v5.21.5~169 (Update
libnet to CPAN version 3.01, 2014-10-10).
Noticed on an Ubuntu system with perl 5.18.2-2ubuntu1.1, which
provides Net::SMTP version 2.31. The error message is
Can't locate object method "starttls" via package "Net::SMTP" at /usr/lib/git-core/git-send-email line 1410.
Reported-by: Brandon Williams <bmwill@google.com> Reported-and-tested-by: Eric Biggers <ebiggers3@gmail.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When "git checkout", "git merge", etc. manipulates the in-core
index, various pieces of information in the index extensions are
discarded from the original state, as it is usually not the case
that they are kept up-to-date and in-sync with the operation on the
main index. The untracked cache extension is copied across these
operations now, which would speed up "git status" (as long as the
cache is properly invalidated).
* dt/unpack-save-untracked-cache-extension:
unpack-trees: preserve index extensions
A recent update to t5545-push-options.sh started skipping all the
tests in the script when a web server testing is disabled or
unavailable, not just the ones that require a web server. Non HTTP
tests have been salvaged to always run in this script.
* jc/skip-test-in-the-middle:
t5545: enhance test coverage when no http server is installed
test: allow skipping the remainder
The recently introduced "[includeIf "gitdir:$dir"] path=..."
mechansim has further been taught to take symlinks into account.
The directory "$dir" specified in "gitdir:$dir" may be a symlink to
a real location, not something that $(getcwd) may return. In such
a case, a realpath of "$dir" is compared with the real path of the
current repository to determine if the contents from the named path
should be included.
* ab/conditional-config-with-symlinks:
config: match both symlink & realpath versions in IncludeIf.gitdir:*
There is no good reason why "git fetch $there $sha1" should fail
when the $sha1 names an object at the tip of an advertised ref,
even when the other side hasn't enabled allowTipSHA1InWant.
* jt/fetch-allow-tip-sha1-implicitly:
fetch-pack: always allow fetching of literal SHA1s
* jh/memihash-opt:
p0004: don't error out if test repo is too small
p0004: don't abort if multi-threaded is too slow
p0004: use test_perf
p0004: avoid using pipes
p0004: simplify calls of test-lazy-init-name-hash
Code from "conversion using external process" codepath has been
extracted to a separate sub-process.[ch] module.
* bp/sub-process-convert-filter:
convert: update subprocess_read_status() to not die on EOF
sub-process: move sub-process functions into separate files
convert: rename reusable sub-process functions
convert: update generic functions to only use generic data structures
convert: separate generic structures and variables from the filter specific ones
convert: split start_multi_file_filter() into two separate functions
pkt-line: annotate packet_writel with LAST_ARG_MUST_BE_NULL
convert: move packet_write_line() into pkt-line as packet_writel()
pkt-line: add packet_read_line_gently()
pkt-line: fix packet_read_line() to handle len < 0 errors
convert: remove erroneous tests for errno == EPIPE
The "run-command" API implementation has been made more robust
against dead-locking in a threaded environment.
* bw/forking-and-threading:
usage.c: drop set_error_handle()
run-command: restrict PATH search to executable files
run-command: expose is_executable function
run-command: block signals between fork and execve
run-command: add note about forking and threading
run-command: handle dup2 and close errors in child
run-command: eliminate calls to error handling functions in child
run-command: don't die in child when duping /dev/null
run-command: prepare child environment before forking
string-list: add string_list_remove function
run-command: use the async-signal-safe execv instead of execvp
run-command: prepare command before forking
t0061: run_command executes scripts without a #! line
t5550: use write_script to generate post-update hook
Simplify parse_pathspec() codepath and stop it from looking at the
default in-core index.
* bw/pathspec-sans-the-index:
pathspec: convert find_pathspecs_matching_against_index to take an index
pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP
ls-files: prevent prune_cache from overeagerly pruning submodules
pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag
submodule: add die_in_unpopulated_submodule function
pathspec: provide a more descriptive die message
"git describe --contains" penalized light-weight tags so much that
they were almost never considered. Instead, give them about the
same chance to be considered as an annotated tag that is the same
age as the underlying commit would.
* jc/name-rev-lw-tag:
name-rev: favor describing with tags and use committer date to tiebreak
name-rev: refactor logic to see if a new candidate is a better name
Tag objects, which are not reachable from any ref, that point at
missing objects were mishandled by "git gc" and friends (they
should silently be ignored instead)
* jk/ignore-broken-tags-when-ignoring-missing-links:
revision.c: ignore broken tags with ignore_missing_links
A test allowed both "git push" and "git receive-pack" on the other
end write their traces into the same file. This is OK on platforms
that allows atomically appending to a file opened with O_APPEND,
but on other platforms led to a mangled output, causing
intermittent test failures. This has been fixed by disabling
traces from "receive-pack" in the test.
* jk/alternate-ref-optim:
t5400: avoid concurrent writes into a trace file
"git interpret-trailers", when used as GIT_EDITOR for "git commit
-v", looked for and appended to a trailer block at the very end,
i.e. at the end of the "diff" output. The command has been
corrected to pay attention to the cut-mark line "commit -v" adds to
the buffer---the real trailer block should appear just before it.
* bm/interpret-trailers-cut-line-is-eom:
interpret-trailers: honor the cut line
"git for-each-ref --format=..." with %(HEAD) in the format used to
resolve the HEAD symref as many times as it had processed refs,
which was wasteful, and "git branch" shared the same problem.
* kn/ref-filter-branch-list:
ref-filter: resolve HEAD when parsing %(HEAD) atom
Git sometimes gives an advice in a rhetorical question that does
not require an answer, which can confuse new users and non native
speakers. Attempt to rephrase them.
* ja/do-not-ask-needless-questions:
git-filter-branch: be more direct in an error message
read-tree -m: make error message for merging 0 trees less smart aleck
usability: don't ask questions if no reply is required
Clarify documentation for include.path and includeIf.<condition>.path
configuration variables.
* jk/doc-config-include:
docs/config: consistify include.path examples
docs/config: avoid the term "expand" for includes
docs/config: give a relative includeIf example
docs/config: clarify include/includeIf relationship
Introduce the BUG() macro to improve die("BUG: ...").
* jk/bug-to-abort:
usage: add NORETURN to BUG() function definitions
config: complain about --local outside of a git repo
setup_git_env: convert die("BUG") to BUG()
usage.c: add BUG() function
Make sure our tests would pass when the sources are checked out
with "platform native" line ending convention by default on
Windows. Some "text" files out tests use and the test scripts
themselves that are meant to be run with /bin/sh, ought to be
checked out with eol=LF even on Windows.
* js/eol-on-ourselves:
t4051: mark supporting files as requiring LF-only line endings
Fix the remaining tests that failed with core.autocrlf=true
t3901: move supporting files into t/t3901/
completion: mark bash script as LF-only
git-new-workdir: mark script as LF-only
Fix build with core.autocrlf=true
"git read-tree -m" (no tree-ish) gave a nonsense suggestion "use
--empty if you want to clear the index". With "-m", such a request
will still fail anyway, as you'd need to name at least one tree-ish
to be merged.
* jc/read-tree-empty-with-m:
read-tree: "read-tree -m --empty" does not make sense
"pack-objects" can stream a slice of an existing packfile out when
the pack bitmap can tell that the reachable objects are all needed
in the output, without inspecting individual objects. This
strategy however would not work well when "--local" and other
options are in use, and need to be disabled.
Conversion from uchar[20] to struct object_id continues.
* bc/object-id: (53 commits)
object: convert parse_object* to take struct object_id
tree: convert parse_tree_indirect to struct object_id
sequencer: convert do_recursive_merge to struct object_id
diff-lib: convert do_diff_cache to struct object_id
builtin/ls-tree: convert to struct object_id
merge: convert checkout_fast_forward to struct object_id
sequencer: convert fast_forward_to to struct object_id
builtin/ls-files: convert overlay_tree_on_cache to object_id
builtin/read-tree: convert to struct object_id
sha1_name: convert internals of peel_onion to object_id
upload-pack: convert remaining parse_object callers to object_id
revision: convert remaining parse_object callers to object_id
revision: rename add_pending_sha1 to add_pending_oid
http-push: convert process_ls_object and descendants to object_id
refs/files-backend: convert many internals to struct object_id
refs: convert struct ref_update to use struct object_id
ref-filter: convert some static functions to struct object_id
Convert struct ref_array_item to struct object_id
Convert the verify_pack callback to struct object_id
Convert lookup_tag to struct object_id
...
* bw/dir-c-stops-relying-on-the-index:
dir: convert fill_directory to take an index
dir: convert read_directory to take an index
dir: convert read_directory_recursive to take an index
dir: convert open_cached_dir to take an index
dir: convert is_excluded to take an index
dir: convert prep_exclude to take an index
dir: convert add_excludes to take an index
dir: convert is_excluded_from_list to take an index
dir: convert last_exclude_matching_from_list to take an index
dir: convert dir_add* to take an index
dir: convert get_dtype to take index
dir: convert directory_exists_in_index to take index
dir: convert read_skip_worktree_file_from_index to take an index
dir: stop using the index compatibility macros
"git checkout --recurse-submodules" did not quite work with a
submodule that itself has submodules.
* sb/checkout-recurse-submodules:
submodule: properly recurse for read-tree and checkout
submodule: avoid auto-discovery in new working tree manipulator code
submodule_move_head: reuse child_process structure for futher commands
* sb/reset-recurse-submodules:
builtin/reset: add --recurse-submodules switch
submodule.c: submodule_move_head works with broken submodules
submodule.c: uninitialized submodules are ignored in recursive commands
entry.c: submodule recursing: respect force flag correctly
grep: assert that threading is enabled when calling grep_{lock,unlock}
Change the grep_{lock,unlock} functions to assert that num_threads is
true, instead of only locking & unlocking the pthread mutex lock when
it is.
These functions are never called when num_threads isn't true, this
logic has gone through multiple iterations since the initial
introduction of grep threading in commit 5b594f457a ("Threaded grep",
2010-01-25), but ever since then they'd only be called if num_threads
was true, so this check made the code confusing to read.
Replace the check with an assertion, so that it's clear to the reader
that this code path is never taken unless we're spawning threads.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
grep: given --threads with NO_PTHREADS=YesPlease, warn
Add a warning about missing thread support when grep.threads or
--threads is set to a non 0 (default) or 1 (no parallelism) value
under NO_PTHREADS=YesPlease.
This is for consistency with the index-pack & pack-objects commands,
which also take a --threads option & are configurable via
pack.threads, and have long warned about the same under
NO_PTHREADS=YesPlease.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a buggy warning about threads under NO_PTHREADS=YesPlease. Due to
re-using the delta_search_threads variable for both the state of the
"pack.threads" config & the --threads option, setting "pack.threads"
but not supplying --threads would trigger the warning for both
"pack.threads" & --threads.
Solve this bug by resetting the delta_search_threads variable in
git_pack_config(), it might then be set by --threads again and be
subsequently warned about, as the test I'm changing here asserts.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pack-objects & index-pack: add test for --threads warning
Add a test for the warning that's emitted when --threads or
pack.threads is provided under NO_PTHREADS=YesPlease. This uses the
new PTHREADS prerequisite.
The assertion for C_LOCALE_OUTPUT in the latter test is currently
redundant, since unlike index-pack the pack-objects warnings aren't
i18n'd. However they might be changed to be i18n'd in the future, and
there's no harm in future-proofing the test.
There's an existing bug in the implementation of pack-objects which
this test currently tests for as-is. Details about the bug & the fix
are included in a follow-up change.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
grep: move is_fixed() earlier to avoid forward declaration
Move the is_fixed() function which are currently only used in
compile_regexp() earlier so it can be used in the PCRE family of
functions in a later change.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
grep: change internal *pcre* variable & function names to be *pcre1*
Change the internal PCRE variable & function names to have a "1"
suffix. This is for preparation for libpcre2 support, where having
non-versioned names would be confusing.
An earlier change in this series ("grep: change the internal PCRE
macro names to be PCRE1", 2017-04-07) elaborates on the motivations
behind this change.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
grep: change the internal PCRE macro names to be PCRE1
Change the internal USE_LIBPCRE define, & build options flag to use a
naming convention ending in PCRE1, without changing the long-standing
USE_LIBPCRE Makefile flag which enables this code.
This is for preparation for libpcre2 support where having things like
USE_LIBPCRE and USE_LIBPCRE2 in any more places than we absolutely
need to for backwards compatibility with old Makefile arguments would
be confusing.
In some ways it would be better to change everything that now uses
USE_LIBPCRE to use USE_LIBPCRE1, and to make specifying
USE_LIBPCRE (or --with-pcre) an error. This would impose a one-time
burden on packagers of git to s/USE_LIBPCRE/USE_LIBPCRE1/ in their
build scripts.
However I'd like to leave the door open to making
USE_LIBPCRE=YesPlease eventually mean USE_LIBPCRE2=YesPlease,
i.e. once PCRE v2 is ubiquitous enough that it makes sense to make it
the default.
This code and the USE_LIBPCRE Makefile argument was added in commit 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09). At the time there was
no indication that the PCRE project would release an entirely new &
incompatible API around 3 years later.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
grep: factor test for \0 in grep patterns into a function
Factor the test for \0 in grep patterns into a function. Since commit 9eceddeec6 ("Use kwset in grep", 2011-08-21) any pattern containing a
\0 is considered fixed as regcomp() can't handle it.
This change makes later changes that make use of either has_null() or
is_fixed() (but not both) smaller.
While I'm at it make the comment conform to the style guide, i.e. add
an opening "/*\n".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove redundant assignments to the "regflags" variable. This variable
is only used set under GREP_PATTERN_TYPE_ERE, so there's no need to
un-set it under GREP_PATTERN_TYPE_{FIXED,BRE,PCRE}.
Back in 5010cb5fcc[1], we did do "opt.regflags &= ~REG_EXTENDED" upon
seeing "-G" on the command line and flipped the bit on upon seeing
"-E", but I think that was perfectly sensible and it would have been a
bug if we didn't. They were part of the command line parsing that
could have seen "-E" on the command line earlier.
When cca2c172 ("git-grep: do not die upon -F/-P when
grep.extendedRegexp is set.", 2011-05-09) switched the command line
parsing to "read into a 'tentatively this is what we saw the last'
variable and then finally commit just once", we didn't touch
opt.regflags for PCRE and FIXED, but we still had to flip regflags
between BRE and ERE, because parsing of grep.extendedregexp
configuration variable directly touched opt.regflags back then, which
was done by b22520a3 ("grep: allow -E and -n to be turned on by
default via configuration", 2011-03-30).
When 84befcd0 ("grep: add a grep.patternType configuration setting",
2012-08-03) introduced extended_regexp_option field, we stopped
flipping regflags while reading the configuration, and that was when
we should have noticed and stopped dropping REG_EXTENDED bit in the
"now we can commit what type to use" helper function.
There is no reason to do this anymore, so stop doing it, more to
reduce "wait this is used under fixed/BRE/PCRE how?" confusion when
reading the code, than to to save ourselves trivial CPU cycles by
removing one assignment.
1. "built-in "git grep"", 2006-04-30.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a die(...) to a default case for the switch statement selecting
between grep pattern types under --recurse-submodules.
Normally this would be caught by -Wswitch, but the grep_pattern_type
type is converted to int by going through parse_options(). Changing
the argument type passed to compile_submodule_options() won't work,
the value will just get coerced. The -Wswitch-default warning will
warn about it, but that produces a lot of noise across the codebase,
this potential issue would be drowned in that noise.
Thus catching this at runtime is the least bad option. This won't ever
trigger in practice, but if a new pattern type were to be added this
catches an otherwise silent bug during development.
See commit 0281e487fd ("grep: optionally recurse into submodules",
2016-12-16) for the initial addition of this code.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
perf: add a comparison test of log --grep regex engines with -F
Add a performance comparison test of log --grepgrep regex engines
given fixed strings.
See the preceding fixed-string t/perf change ("perf: add a comparison
test of grep regex engines with -F", 2017-04-21) for notes about this,
in particular this mostly tests exactly the same codepath now, but
might not in the future:
perf: add a comparison test of log --grep regex engines
Add a very basic performance comparison test comparing the POSIX
basic, extended and perl engines with patterns matching log messages
via --grep=<pattern>.
See commit ("perf: add a comparison test of grep regex engines",
2017-04-19) for details on the machine the above test run was executed
on.
Before commit ("log: make --regexp-ignore-case work with
--perl-regexp", 2017-05-20) this test will almost definitely
fail (depending on the repo) if passed the -i option, since it wasn't
properly supported under PCRE.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
perf: add a comparison test of grep regex engines with -F
Add a performance comparison test of grep regex engines given fixed
strings.
The current logic in compile_regexp() ignores the engine parameter and
uses kwset() to search for these, so this test shows no difference
between engines right now:
$ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux ./run p7821-grep-engines-fixed.sh
[...]
Test this tree
------------------------------------------------
7821.1: fixed grep int 0.56(1.67+0.68)
7821.2: basic grep int 0.57(1.70+0.57)
7821.3: extended grep int 0.59(1.76+0.51)
7821.4: perl grep int 1.08(1.71+0.55)
7821.6: fixed grep uncommon 0.23(0.55+0.50)
7821.7: basic grep uncommon 0.24(0.55+0.50)
7821.8: extended grep uncommon 0.26(0.55+0.52)
7821.9: perl grep uncommon 0.24(0.58+0.47)
7821.11: fixed grep æ 0.36(1.30+0.42)
7821.12: basic grep æ 0.36(1.32+0.40)
7821.13: extended grep æ 0.38(1.30+0.42)
7821.14: perl grep æ 0.35(1.24+0.48)
Only when run with -i via GIT_PERF_7821_GREP_OPTS=' -i' do we avoid
avoid going through the same kwset.[ch] codepath, see the "Even when
-F..." comment in grep.c. This only kicks for the non-ASCII case:
Add a very basic performance comparison test comparing the POSIX
basic, extended and perl engines.
In theory the "basic" and "extended" engines should be implemented
using the same underlying code with a slightly different pattern
parser, but some implementations may not do this. Jump through some
slight hoops to test both, which is worthwhile since "basic" is the
default.
Running this on an i7 3.4GHz Linux 4.9.0-2 Debian testing against a
checkout of linux.git & latest upstream PCRE, both PCRE and git
compiled with -O3 using gcc 7.1.1:
Options can also be passed to git-grep via the GIT_PERF_7820_GREP_OPTS
environment variable. There are various modes such as "-v" that have
very different performance profiles, but handling the combinatorial
explosion of testing all those options would make this script much
more complex and harder to maintain. Instead just add the ability to
do one-shot runs with arbitrary options, e.g.:
Windows: do not treat a path with backslashes as a remote's nick name
On Windows, the remote repository name in, e.g., `git fetch foo\bar`
is clearly not a nickname for a configured remote repository. However,
the function valid_remote_nick() does not account for backslashes.
Use is_dir_sep() to check for both slashes and backslashes on Windows.
This was discovered while playing with Duy's patches that warn after
fopen() failures. The functions that read the branches and remotes
files are protected by a valid_remote_nick() check. Without this
change, a Windows style absolute path is incorrectly regarded as
nickname and is concatenated to a prefix and used with fopen(). This
triggers warnings because a colon in a path name is not allowed:
C:\Temp\gittest>git fetch C:\Temp\gittest
warning: unable to access '.git/remotes/C:\Temp\gittest': Invalid argument
warning: unable to access '.git/branches/C:\Temp\gittest': Invalid argument
From C:\Temp\gittest
* branch HEAD -> FETCH_HEAD
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we diff a blob against a working tree file like:
git diff HEAD:Makefile Makefile
we always use the working tree filename for both sides of
the diff. In most cases that's fine, as the two would be the
same anyway, as above. And until recently, we used the
"name" for the blob, not the path, which would have the
messy "HEAD:" on the beginning.
But when they don't match, like:
git diff HEAD:old_path new_path
it makes sense to show both names.
This patch uses the blob's path field if it's available, and
otherwise falls back to using the filename (in preference to
the blob's name, which is likely to be garbage like a raw
sha1).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There's a subtle distinction between "name" and "path" for a
blob that we resolve: the name is what the user told us on
the command line, and the path is what we traversed when
finding the blob within a tree (if we did so).
When we diff blobs directly, we use "name", but "path" is
more likely to be useful to the user (it will find the
correct .gitattributes, and give them a saner diff header).
We still have to fall back to using the name for some cases
(i.e., any blob reference that isn't of the form tree:path).
That's the best we can do in such a case.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff: use the word "path" instead of "name" for blobs
The stuff_change() function makes diff_filespecs out of
blobs. The term we generally use for filespecs is "path",
not "name", so let's be consistent here. That will make
things less confusing when the next patch starts caring
about the path/name distinction inside the pending object
array.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When diffing blobs directly, git-diff picks the blobs out of
the rev_info's pending array and copies the relevant bits to
a custom "struct blobinfo". But the pending array entry
already has all of this information (and more, which we'll
use in future patches). Let's just pass the original entry
instead.
In practice, these two blobs are probably adjacent in the
revs->pending array, and we could just pass the whole array.
But the current code is careful to pick each blob out
separately and put it into another array, so we'll continue
to do so and make our own array-of-pointers.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
handle_revision_arg: record paths for pending objects
If the revision parser sees an argument like tree:path, we
parse it down to the correct blob (or tree), but throw away
the "path" portion. Let's ask get_sha1_with_context() to
record it, and pass it along in the pending array.
This will let programs like git-diff which rely on the
revision-parser show more accurate paths.
Note that the implementation is a little tricky; we have to
make sure we free oc.path in all code paths. For handle_dotdot(),
we can piggy-back on the existing cleanup-wrapper pattern.
The real work happens in handle_dotdot_1(), but the
handle_dotdot() wrapper makes sure that the path is freed no
matter how we exit the function (and for that reason we make
sure that the object_context struct is zero'd, so if we fail
to even get to the get_sha1_with_context() call, we just end
up calling free(NULL)).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
handle_revision_arg: record modes for "a..b" endpoints
The "a..b" revision syntax was designed to handle commits,
so it doesn't bother to record any mode we find while
traversing a "tree:path" endpoint. These days "git diff" can
diff blobs using either "a:path..b:path" (with dots) or
"a:path b:path" (without), but the two behave
inconsistently, as the with-dots version fails to notice the
mode.
Let's teach the dot-dot range parser to record modes; it
doesn't cost us anything, and it makes this case work.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The git-diff command can directly compare two blobs (or a
blob and a file), but we don't test this at all. Let's add
some basic tests that reveal a few problems.
There are basically four interesting inputs:
1. sha1 against sha1 (where diff has no information beyond
the contents)
2. tree:path against tree:path (where it can get
information via get_sha1_with_context)
3. Same as (2), but using the ".." range syntax
4. tree:path against a filename
And beyond generating a sane diff, we care about a few
little bits: which paths they show in the diff header, and
whether they correctly pick up a mode change.
They should all be able to show a mode except for (1),
though note that case (3) is currently broken.
For the headers, we would ideally show the path within the
tree if we have it, making:
git diff a:path b:path
look the same as:
git diff a b -- path
We can't always do that (e.g., in the direct sha1/sha1 diff,
we have no path to show), in which case we should fall back
to the name that resolved to the blob (which is nonsense
from the repository's perspective, but is the best we can
do).
Aside from the fallback case in (1), none of the cases get
this right. Cases (2) and (3) always show the full
tree:path, even though we should be able to know just the
"path" portion.
Case (4) picks up the filename path, but assigns it to
_both_ sides of the diff. So this works for:
git diff tree:path path
but not for:
git diff tree:other_path path
The appropriate tests are marked to expect failure.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a sha1 lookup returns the tree path via "struct
object_context", it just copies it into a fixed-size buffer.
This means the result can be truncated, and it means our
"struct object_context" consumes a lot of stack space.
Instead, let's allocate a string on the heap. Because most
callers don't care about this information, we'll avoid doing
it by default (so they don't all have to start calling
free() on the result). There are basically two options for
the caller to signal to us that it's interested:
1. By setting a pointer to storage in the object_context.
2. By passing a flag in another parameter.
Doing (1) would match the way that sha1_object_info_extended()
works. But it would mean that every caller would have to
initialize the object_context, which they don't currently
have to do.
This patch does (2), and adds a new bit to the function's
flags field. All of the callers that look at the "path"
field are updated to pass the new flag.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The get_sha1_with_context() function zeroes out the
oc->symlink_path strbuf, but doesn't use strbuf_init() to
set up the usual invariants (like pointing to the slopbuf).
We don't actually write to the oc->symlink_path strbuf
unless we call get_tree_entry_follow_symlinks(), and that
function does initialize it. However, readers may still look
at the zero'd strbuf.
In practice this isn't a triggerable bug. The only caller
that looks at it only does so when the mode we found is 0.
This doesn't happen for non-tree-entries (where we return
S_IFINVALID). A broken tree entry could have a mode of 0,
but canon_mode() quietly rewrites that into S_IFGITLINK.
So the "0" mode should only come up when we did indeed find
a symlink.
This is mostly just an accident of how the code happens to
work, though. Let's future-proof ourselves to make sure the
strbuf is properly initialized for all calls (it's only a
few struct member assignments, not a heap allocation).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
sha1_name: consistently refer to object_context as "oc"
An early version of the patch to add object_context used the
name object_resolve_context. This was later shortened to
just object_context, but the "orc" variable name stuck in a
few places. Let's use "oc", which is used elsewhere in the
code.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The handle_revision_arg function is rather long, and a big
chunk of it is handling the range operators. Let's pull that
out to a separate helper. While we're doing so, we can clean
up a few of the rough edges that made the flow hard to
follow:
- instead of manually restoring *dotdot (that we overwrote
with a NUL), do the real work in a sub-helper, which
makes it clear that the munge/restore lines are a
matched pair
- eliminate a goto which wasn't actually used for control
flow, but only to avoid duplicating a few lines
(instead, those lines are pushed into another helper
function)
- use early returns instead of deep nesting
- consistently name all variables for the left-hand side
of the range as "a" (rather than "this" or "from") and
the right-hand side as "b" (rather than "next", or using
the unadorned "sha1" or "flags" from the main function).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
handle_revision_arg: hoist ".." check out of range parsing
Since 003c84f6d (specifying ranges: we did not mean to make
".." an empty set, 2011-05-02), we treat the argument ".."
specially. We detect it by noticing that both sides of the
range are empty, and that this is a non-symmetric two-dot
range.
While correct, this makes the code overly complicated. We
can just detect ".." up front before we try to do further
parsing. This avoids having to de-munge the NUL from dotdot,
and lets us eliminate an extra const array (which we needed
only to do direct pointer comparisons).
It also removes the one code path from the range-parsing
conditional that requires us to return -1. That will make it
simpler to pull the dotdot parsing out into its own
function.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
handle_revision_arg: stop using "dotdot" as a generic pointer
The handle_revision_arg() function has a "dotdot" variable
that it uses to find a ".." or "..." in the argument. If we
don't find one, we look for other marks, like "^!". But we
just keep re-using the "dotdot" variable, which is
confusing.
Let's introduce a separate "mark" variable that can be used
for these other marks. They still reuse the same variable,
but at least the name is no longer actively misleading.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "dotdot" range parser avoids calling
lookup_commit_reference() if we are directly fed two
commits. But its casts are unnecessarily complex; that
function will just return a commit we pass into it.
Just calling the function all the time is much simpler, and
doesn't do any significant extra work (the object is already
parsed, and deref_tag() on a non-tag is a noop; we do incur
one extra lookup_object() call, but that's fairly trivial).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we are parsing a range like "a..b", we write a
temporary NUL over the first ".", so that we can access the
names "a" and "b" as C strings. But our restoration of the
original "." is done at inconsistent times, which can lead
to confusing results.
For most calls, we restore the "." after we resolve the
names, but before we call verify_non_filename(). This means
that when we later call add_pending_object(), the name for
the left-hand "a" has been re-expanded to "a..b". You can
see this with:
git log --source a...b
where "b" will be correctly marked with "b", but "a" will be
marked with "a...b". Likewise with "a..b" (though you need
to use --boundary to even see "a" at all in that case).
To top off the confusion, when the REVARG_CANNOT_BE_FILENAME
flag is set, we skip the non-filename check, and leave the
NUL in place.
That means we do report the correct name for "a" in the
pending array. But some code paths try to show the whole
"a...b" name in error messages, and these erroneously show
only "a" instead of "a...b". E.g.:
(That last message should be "HEAD:foo...HEAD:foo"; I used
cherry-pick because it passes the CANNOT_BE_FILENAME flag).
As an interesting side note, cherry-pick actually looks at
and re-resolves the arguments from the pending->name fields.
So it would have been visibly broken by the first bug, but
the effect was canceled out by the second one.
This patch makes the whole function consistent by re-writing
the NUL immediately after calling verify_non_filename(), and
then restoring the "." as appropriate in some error-printing
and early-return code paths.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is an implicit assumption that a directory containing only
untracked and ignored paths should itself be considered untracked. This
makes sense in use cases where we're asking if a directory should be
added to the git database, but not when we're asking if a directory can
be safely removed from the working tree; as a result, clean -d would
assume that an "untracked" directory containing ignored paths could be
deleted, even though doing so would also remove the ignored paths.
To get around this, we teach clean -d to collect ignored paths and skip
an untracked directory if it contained an ignored path, instead just
removing the untracked contents thereof. To achieve this, cmd_clean()
has to collect all untracked contents of untracked directories, in
addition to all ignored paths, to determine which untracked dirs must be
skipped (because they contain ignored paths) and which ones should *not*
be skipped.
For this purpose, correct_untracked_entries() is introduced to prune a
given dir_struct of untracked entries containing ignored paths and those
untracked entries encompassed by the untracked entries which are not
pruned away.
A memory leak is also fixed in cmd_clean().
This also fixes the known breakage in t7300, since clean -d now skips
untracked directories containing ignored paths.
Signed-off-by: Samuel Lijin <sxlijin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
On Windows the environment variable PATH contains a semicolon-separated
list of directories to search for, in order, when looking for the
location of a binary to run. get_path_split() parses it and returns an
array of string copies, which is iterated by path_lookup(), which in
turn passes each entry to lookup_prog().
Change lookup_prog() to take the directory name as a length-limited
string instead of as a NUL-terminated one and parse PATH directly in
path_lookup(). This avoids memory allocations, simplifying the code.
Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
mingw.h: permit arguments with side effects for is_dir_sep
Taking git-compat-util.h's cue (which uses an inline function to back
is_dir_sep()), let's use an inline function to back also the Windows
version of is_dir_sep(). This avoids problems when calling the function
with arguments that do more than just provide a single character, e.g.
incrementing a pointer. Example:
is_dir_sep(*p++)
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Documentation: fix reference to ifExists for interpret-trailers
The manual for "git interpret-trailers" mentioned a non-existing
literal `overwrite` for its config option `trailer.ifexists`. The
correct name for that choice is `replace`.
Signed-off-by: Andreas Heiduk <asheiduk@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The description of the test was not enclosed in single quotes, which
broke the coloring scheme that I am used to. Upon closer inspection
the test is good, but the description is a bit vague. So extend the
description of the first test.
While at it align the description of the file to match what we actually
test in the file.
Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update tests to pass under GETTEXT_POISON (a mechanism to ensure
that output strings that should not be translated are not
translated by mistake), and tell TravisCI to run them.
* ab/fix-poison-tests:
travis-ci: add job to run tests with GETTEXT_POISON
travis-ci: setup "prove cache" in "script" step
tests: fix tests broken under GETTEXT_POISON=YesPlease
The Web interface to gmane news archive is long gone, even though
the articles are still accessible via NTTP. Replace the links with
ones to public-inbox.org. Because their message identification is
based on the actual message-id, it is likely that it will be easier
to migrate away from it if/when necessary.
* ab/doc-replace-gmane-links:
doc: replace more gmane links
doc: replace a couple of broken gmane links
A few codepaths in "checkout" and "am" working on an unborn branch
tried to access an uninitialized piece of memory.
* rs/checkout-am-fix-unborn:
am: check return value of resolve_refdup before using hash
checkout: check return value of resolve_refdup before using hash
* ls/travis-relays-for-windows-ci:
travis-ci: retry if Git for Windows CI returns HTTP error 502 or 503
travis-ci: handle Git for Windows CI status "failed" explicitly