In d8193743e0 (usage.c: add BUG() function, 2017-05-12), a new macro
was introduced to use for reporting bugs instead of die(). It was then
subsequently used to convert one single caller in 588a538ae5
(setup_git_env: convert die("BUG") to BUG(), 2017-05-12).
The cover letter of the patch series containing this patch
(cf 20170513032414.mfrwabt4hovujde2@sigill.intra.peff.net) is not
terribly clear why only one call site was converted, or what the plan
is for other, similar calls to die() to report bugs.
Let's just convert all remaining ones in one fell swoop.
This trick was performed by this invocation:
sed -i 's/die("BUG: /BUG("/g' $(git grep -l 'die("BUG' \*.c)
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The final step to make an empty string as a pathspec element
illegal. We started this by first deprecating and warning a
pathspec that has such an element in 2.11 (Nov 2016).
Hopefully we can merge this down to the 'master' by the end of the
year? A deprecation warning period that is about 1 year does not
sound too bad.
* ex/deprecate-empty-pathspec-as-match-all:
pathspec: die on empty strings as pathspec
t0027: do not use an empty string as a pathspec element
The command line arguments passed to main() are valid for the life of
a program, but the same is not true for all other argv-style arrays
(e.g. when a caller creates an argv_array). Clarify that
parse_pathspec does not rely on the argv passed to it to remain valid.
This makes it easier to tell that callers like "git rev-list --stdin"
are safe and ensures that that is more likely to remain true as the
implementation of parse_pathspec evolves.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A common pattern to free a piece of memory and assign NULL to the
pointer that used to point at it has been replaced with a new
FREE_AND_NULL() macro.
* ab/free-and-null:
*.[ch] refactoring: make use of the FREE_AND_NULL() macro
coccinelle: make use of the "expression" FREE_AND_NULL() rule
coccinelle: add a rule to make "expression" code use FREE_AND_NULL()
coccinelle: make use of the "type" FREE_AND_NULL() rule
coccinelle: add a rule to make "type" code use FREE_AND_NULL()
git-compat-util: add a FREE_AND_NULL() wrapper around free(ptr); ptr = NULL
Fix configuration codepath to pay proper attention to commondir
that is used in multi-worktree situation, and isolate config API
into its own header file.
* bw/config-h:
config: don't implicitly use gitdir or commondir
config: respect commondir
setup: teach discover_git_directory to respect the commondir
config: don't include config.h by default
config: remove git_config_iter
config: create config.h
An empty string as a pathspec element matches all paths. A buggy
script, however, could accidentally assign an empty string to a
variable that then gets passed to a Git command invocation, e.g.:
path=... compute a path to be removed in $path ...
git rm -r "$path"
which would unintentionally remove all paths in the current
directory.
The fix for this issue comprises of two steps. Step 1, which warns
that empty strings as pathspecs will become invalid, has already
been implemented in commit d426430 ("pathspec: warn on empty strings
as pathspec", 2016-06-22).
This patch is step 2. It removes the warning and throws an error
instead.
Signed-off-by: Emily Xie <emilyxxie@gmail.com>
Reported-by: David Turner <novalis@novalis.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the result of the just-added coccinelle rule. This manually
excludes a few occurrences, mostly things that resulted in many
FREE_AND_NULL() on one line, that'll be manually fixed in a subsequent
change.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Stop including config.h by default in cache.h. Instead only include
config.h in those files which require use of the config system.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert find_pathspecs_matching_against_index to take an index
parameter.
In addition mark pathspec.c with NO_THE_INDEX_COMPATIBILITY_MACROS now
that it doesn't use any cache macros or reference 'the_index'.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since (ae8d08242 pathspec: pass directory indicator to
match_pathspec_item()) the path matching logic has been able to cope
with submodules without needing to strip off a trailing slash if a path
refers to a submodule.
Since stripping the slash is no longer necessary, remove the
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since (ae8d08242 pathspec: pass directory indicator to
match_pathspec_item()) the path matching logic has been able to cope
with submodules without needing to strip off a trailing slash if a path
refers to a submodule.
Since the stripping the trailing slash is no longer necessary, remove
the PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag. In addition, factor
out the logic which dies if a path decends into a submodule so that it
can still be used as a check after a pathspec struct has been
initialized.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently 'git add' is the only command which dies when launched from an
unpopulated submodule (the place-holder directory for a submodule which
hasn't been checked out). This is triggered implicitly by passing the
PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag to 'parse_pathspec()'.
Instead make this desire more explicit by creating a function
'die_in_unpopulated_submodule()' which dies if the provided 'prefix' has
a leading path component which matches a submodule in the the index.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current message displayed upon an internal error in
'init_pathspec_item()' isn't very descriptive and doesn't provide much
context to where the error occurred. Update the error message to
provide more context to where the error occured.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A recent update broke "git add -p ../foo" from a subdirectory.
* ps/pathspec-empty-prefix-origin:
pathspec: honor `PATHSPEC_PREFIX_ORIGIN` with empty prefix
In 'clear_pathspec()' the incorrect index parameter is used to bound an
inner-loop which is used to free a 'struct attr_match' value field.
Using the incorrect index parameter (in addition to being incorrect)
occasionally causes segmentation faults when attempting to free an
invalid pointer. Fix this by using the correct index parameter 'i'.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previous to commit 5d8f084a5 (pathspec: simpler logic to prefix original
pathspec elements, 2017-01-04), we were always using the computed
`match` variable to perform pathspec matching whenever
`PATHSPEC_PREFIX_ORIGIN` is set. This is for example useful when passing
the parsed pathspecs to other commands, as the computed `match` may
contain a pathspec relative to the repository root. The commit changed
this logic to only do so when we do have an actual prefix and when
literal pathspecs are deactivated.
But this change may actually break some commands which expect passed
pathspecs to be relative to the repository root. One such case is `git
add --patch`, which now fails when using relative paths from a
subdirectory. For example if executing "git add -p ../foo.c" in a
subdirectory, the `git-add--interactive` command will directly pass
"../foo.c" to `git-ls-files`. As ls-files is executed at the
repository's root, the command will notice that "../foo.c" is outside
the repository and fail.
Fix the issue by again using the computed `match` variable when
`PATHSPEC_PREFIX_ORIGIN` is set and global literal pathspecs are
deactivated. Note that in contrast to previous behavior, we will now
always call `prefix_magic` regardless of whether a prefix is actually
set. But this is the right thing to do: when the `match` variable has
been resolved to the repository's root, it will be set to an empty
string. When passing the empty string directly to other commands, it
will result in a warning regarding deprecated empty pathspecs. By always
adding the prefix magic, we will end up with at least the string
":(prefix:0)" and thus avoid the warning.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Acked-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Duy Nguyen <pclouds@gmail.com>
In our own .gitattributes file we have attributes such as:
*.[ch] whitespace=indent,trail,space
When querying for attributes we want to be able to ask for the exact
value, i.e.
git ls-files :(attr:whitespace=indent,trail,space)
should work, but the commas are used in the attr magic to introduce
the next attr, such that this query currently fails with
fatal: Invalid pathspec magic 'trail' in ':(attr:whitespace=indent,trail,space)'
This change allows escaping characters by a backslash, such that the query
git ls-files :(attr:whitespace=indent\,trail\,space)
will match all path that have the value "indent,trail,space" for the
whitespace attribute. To accomplish this, we need to modify two places.
First `parse_long_magic` needs to not stop early upon seeing a comma or
closing paren that is escaped. As a second step we need to remove any
escaping from the attr value.
Based on a patch by Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The pathspec mechanism is extended via the new
":(attr:eol=input)pattern/to/match" syntax to filter paths so that it
requires paths to not just match the given pattern but also have the
specified attrs attached for them to be chosen.
Based on a patch by Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of erroring out and telling the user that they should add a
positive pattern that covers everything else, just _do_ that.
For commands where we honor the current cwd by default (ie grep, ls-files
etc), we make that default positive pathspec be the current working
directory. And for commands that default to the whole project (ie diff,
log, etc), the default positive pathspec is the whole project.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The choice of '!' for a negative pathspec ends up not only not matching
what we do for revisions, it's also a horrible character for shell
expansion since it needs quoting.
So add '^' as an alternative alias for an excluding pathspec entry.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Every once in a while someone complains to the mailing list to have
run into this weird assertion[1]. The usual response from the mailing
list is link to old discussions[2], and acknowledging the problem
stating it is known.
This patch accomplishes two things:
1. Switch assert() to die("BUG") to give a more readable message.
2. Take one of the cases where we hit a BUG and turn it into a normal
"there was something wrong with the input" message.
This assertion triggered for cases where there wasn't a programming
bug, but just bogus input. In particular, if the user asks for a
pathspec that is inside a submodule, we shouldn't assert() or
die("BUG"); we should tell the user their request is bogus.
The only reason we did not check for it, is the expensive nature
of such a check, so callers avoid setting the flag
PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE. However when we die due
to bogus input, the expense of CPU cycles spent outweighs the user
wondering what went wrong, so run that check unconditionally before
dying with a more generic error message.
Note: There is a case (e.g. "git -C submodule add .") in which we call
strip_submodule_slash_expensive, as git-add requests it via the flag
PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE, but the assert used to
trigger nevertheless, because the flag PATHSPEC_LITERAL was not set,
such that we executed
if (item->nowildcard_len < prefixlen)
item->nowildcard_len = prefixlen;
and prefixlen was not adapted (e.g. it was computed from "submodule/")
So in the die_inside_submodule_path function we also need handle paths,
that were stripped before, i.e. are the exact submodule path. This
is why the conditions in die_inside_submodule_path are slightly
different than in strip_submodule_slash_expensive.
[1] https://www.google.com/search?q=item-%3Enowildcard_len
[2] http://git.661346.n2.nabble.com/assert-failed-in-submodule-edge-case-td7628687.htmlhttps://www.spinics.net/lists/git/msg249473.html
Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Give a more relevant name to the prefix_pathspec function as it does
more than just prefix a pathspec element.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A few small changes to improve readability. This is done by grouping related
assignments, adding blank lines, ensuring lines are <80 characters, and
adding additional comments.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Factor out the logic responsible for stripping the trailing slash on
pathspecs referencing submodules into its own function.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Factor out the logic responsible for the magic in a pathspec element
into its own function.
Also avoid calling into the parsing functions when
`PATHSPEC_LITERAL_PATH` is specified since it causes magic to be
ignored and all paths to be treated as literals.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Factor out the logic responsible for parsing long magic into its own
function. As well as hoist the prefix check logic outside of the inner
loop as there isn't anything that needs to be done after matching
"prefix:".
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Factor out the logic responsible for parsing short magic into its own
function.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Create helper functions to read the global magic environment variables
in additon to factoring out the global magic gathering logic into its
own function.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The logic used to prefix an original pathspec element with 'prefix'
magic is more general purpose and can be used for more than just short
magic. Remove the extra code paths and rename 'prefix_short_magic' to
'prefix_magic' to better indicate that it can be used in more general
situations.
Also, slightly change the logic which decides when to prefix the
original element in order to prevent a pathspec of "." from getting
converted to "" (empty string).
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
For better clarity, always show the mnemonic and name of the unsupported
magic being used. This lets users have a more clear understanding of
what magic feature isn't supported. And if they supplied a mnemonic,
the user will be told what its corresponding name is which will allow
them to more easily search the man pages for that magic type.
This also avoids passing an extra parameter around the pathspec
initialization code.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Removed unused variable 'n' from the 'unsupported_magic()' function.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The 'original' string entry in a pathspec_item is only duplicated some
of the time, instead always make a copy of the original and take
ownership of the memory.
Since both 'match' and 'original' string entries in a pathspec_item are
owned by the pathspec struct, they need to be freed when clearing the
pathspec struct (in 'clear_pathspec()') and duplicated when copying the
pathspec struct (in 'copy_pathspec()').
Also change the type of 'match' and 'original' to 'char *' in order to
more explicitly show the ownership of the memory.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now that all callers of the old 'get_pathspec' interface have been
migrated to use the new pathspec struct interface it can be removed
from the codebase.
Since there are no more users of the '_raw' field in the pathspec struct
it can also be removed. This patch also removes the old functionality
of modifying the const char **argv array that was passed into
parse_pathspec. Instead the constructed 'match' string (which is a
pathspec element with the prefix prepended) is only stored in its
corresponding pathspec_item entry.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
An empty string used as a pathspec element has always meant
'everything matches', but it is too easy to write a script that
finds a path to remove in $path and run 'git rm "$paht"', which
ends up removing everything. Start warning about this use of an
empty string used for 'everything matches' and ask users to use a
more explicit '.' for that instead.
The hope is that existing users will not mind this change, and
eventually the warning can be turned into a hard error, upgrading
the deprecation into removal of this (mis)feature.
* ex/deprecate-empty-pathspec-as-match-all:
pathspec: warn on empty strings as pathspec
We call "qsort(array, nelem, sizeof(array[0]), fn)", and most of
the time third parameter is redundant. A new QSORT() macro lets us
omit it.
* rs/qsort:
show-branch: use QSORT
use QSORT, part 2
coccicheck: use --all-includes by default
remove unnecessary check before QSORT
use QSORT
add QSORT
Apply the semantic patch contrib/coccinelle/qsort.cocci to the code
base, replacing calls of qsort(3) with QSORT. The resulting code is
shorter and supports empty arrays with NULL pointers.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a semantic patch for converting certain calls of memcpy(3) to
COPY_ARRAY() and apply that transformation to the code base. The result
is
shorter and safer code. For now only consider calls where source and
destination have the same type, or in other words: easy cases.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
An empty string as a pathspec element matches all paths. A buggy
script, however, could accidentally assign an empty string to a
variable that then gets passed to a Git command invocation, e.g.:
path=... compute a path to be removed in $path ...
git rm -r "$paht"
which would unintentionally remove all paths in the current
directory.
The fix for this issue requires a two-step approach. As there may be
existing scripts that knowingly use empty strings in this manner,
the first step simply gives a warning that (1) tells that an empty
string will become an invalid pathspec element and (2) asks the user
to use "." if they mean to match all.
For step two, a follow-up patch several release cycles later will
remove the warning and throw an error instead.
This patch is the first step.
Signed-off-by: Emily Xie <emilyxxie@gmail.com>
Reported-by: David Turner <novalis@novalis.org>
Mentored-by: Michail Denchev <mdenchev@gmail.com>
Thanks-to: Sarah Sharp <sarah@thesharps.us> and James Sharp <jamey@minilop.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function takes a pointer to a pathspec structure, and releases
the resources held by it, but does not free() the structure itself.
Such a function should be called "clear", not "free".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Each of these cases can be converted to use ALLOC_ARRAY or
REALLOC_ARRAY, which has two advantages:
1. It automatically checks the array-size multiplication
for overflow.
2. It always uses sizeof(*array) for the element-size,
so that it can never go out of sync with the declared
type of the array.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use xcalloc() instead of xmalloc() followed by memset() to allocate
and zero out memory because it's shorter and avoids duplicating the
function parameters.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This helps reduce the number of match_pathspec_depth() call sites and
show how match_pathspec_depth() is used.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Introduce "negative pathspec" magic, to allow "git log -- . ':!dir'" to
tell us "I am interested in everything but 'dir' directory".
* nd/negative-pathspec:
pathspec.c: support adding prefix magic to a pathspec with mnemonic magic
Support pathspec magic :(exclude) and its short form :!
glossary-content.txt: rephrase magic signature part
Remove a few duplicate implementations of prefix/suffix comparison
functions, and rename them to starts_with and ends_with.
* cc/starts-n-ends-with:
replace {pre,suf}fixcmp() with {starts,ends}_with()
strbuf: introduce starts_with() and ends_with()
builtin/remote: remove postfixcmp() and use suffixcmp() instead
environment: normalize use of prefixcmp() by removing " != 0"
Back in 233c3e6 (parse_pathspec: preserve prefix length via
PATHSPEC_PREFIX_ORIGIN - 2013-07-14), parse_pathspec() is taught to
save prefix length as a dynamic magic. This is needed when the
pathspec is passed to another process and and prefix lenght would be
lost.
Back then we support two cases. If the pathspec is normal, e.g. "abc",
we simply add the prefix to become ":(prefix:2)abc". If the pathspec
contains long magic, e.g. ":(foo,bar)abc" then we turn it to
":(foo,bar,prefix:2)abc". We do not support prefixing on short form,
because the only supported mnemonic '/' disappears after the the
preprocessing steps.
With the introduction of exclude magic with mnemonic '!', we need to
add support for the short form case so that ':!abc' becomes
':(exclude,prefix:2)abc'. Without this, it will break
cd Documentation
git add -p -- . ':!technical'
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Leaving only the function definitions and declarations so that any
new topic in flight can still make use of the old functions, replace
existing uses of the prefixcmp() and suffixcmp() with new API
functions.
The change can be recreated by mechanically applying this:
$ git grep -l -e prefixcmp -e suffixcmp -- \*.c |
grep -v strbuf\\.c |
xargs perl -pi -e '
s|!prefixcmp\(|starts_with\(|g;
s|prefixcmp\(|!starts_with\(|g;
s|!suffixcmp\(|ends_with\(|g;
s|suffixcmp\(|!ends_with\(|g;
'
on the result of preparatory changes in this series.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Schindelin
Junio C Hamano
Jonathan Nieder
Ville Skyttä
Emily Xie
Ævar Arnfjörð Bjarmason
Brandon Williams
Patrick Steinhardt
Linus Torvalds
Stefan Beller
René Scharfe
Jeff King
Nguyễn Thái Ngọc Duy
Christian Couder