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>
read-tree is basically the front end of unpack-trees code and shoud
expose all of its functionality (unless it's designed for internal
use). This "opts.quiet" (formerly "opts.gently") was added for
builtin/checkout.c but there is no reason why other read-tree users
won't find this useful.
The test that is updated to run 'read-tree --quiet' was added because
unpack-trees was accidentally not being quiet [1] in 6a143aa2b2
(checkout -m: attempt merge when deletion of path was staged -
2014-08-12). Because checkout is the only "opts.quiet" user, there was
no other way to test quiet behavior. But we can now test it directly.
6a143aa2b2 was manually reverted to verify that read-tree --quiet
works correctly (i.e. test_must_be_empty fails).
[1] the commit message there say "errors out instead of performing a
merge" but I'm pretty sure the "performing a merge" happens anyway
even before that commit. That line should say "errors out
_in addition to_ performing a merge"
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
By default, index compat macros are off from now on, because they
could hide the_index dependency.
Only those in builtin can use it.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This case is more interesting than other boring "remove the_repo"
commits because while we need access to the object database, we cannot
simply use r->index because unpack-trees.c can operate on a temporary
index, not $GIT_DIR/index. Ideally we should be able to pass an object
database to lookup_tree() but that ship has sailed.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we define a parse-options callback, the flags we put in the option
struct must match what the callback expects. For example, a callback
which does not handle the "unset" parameter should only be used with
PARSE_OPT_NONEG. But since the callback and the option struct are not
defined next to each other, it's easy to get this wrong (as earlier
patches in this series show).
Fortunately, the compiler can help us here: compiling with
-Wunused-parameters can show us which callbacks ignore their "unset"
parameters (and likewise, ones that ignore "arg" expect to be triggered
with PARSE_OPT_NOARG).
But after we've inspected a callback and determined that all of its
callers use the right flags, what do we do next? We'd like to silence
the compiler warning, but do so in a way that will catch any wrong calls
in the future.
We can do that by actually checking those variables and asserting that
they match our expectations. Because this is such a common pattern,
we'll introduce some helper macros. The resulting messages aren't
as descriptive as we could make them, but the file/line information from
BUG() is enough to identify the problem (and anyway, the point is that
these should never be seen).
Each of the annotated callbacks in this patch triggers
-Wunused-parameters, and was manually inspected to make sure all callers
use the correct options (so none of these BUGs should be triggerable).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The parse-options machinery learned to refrain from enclosing
placeholder string inside a "<bra" and "ket>" pair automatically
without PARSE_OPT_LITERAL_ARGHELP. Existing help text for option
arguments that are not formatted correctly have been identified and
fixed.
* rs/parse-opt-lithelp:
parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP
shortlog: correct option help for -w
send-pack: specify --force-with-lease argument help explicitly
pack-objects: specify --index-version argument help explicitly
difftool: remove angular brackets from argument help
add, update-index: fix --chmod argument help
push: use PARSE_OPT_LITERAL_ARGHELP instead of unbalanced brackets
Parseopt wraps argument help strings in a pair of angular brackets by
default, to tell users that they need to replace it with an actual
value. This is useful in most cases, because most option arguments
are indeed single values of a certain type. The option
PARSE_OPT_LITERAL_ARGHELP needs to be used in option definitions with
arguments that have multiple parts or are literal strings.
Stop adding these angular brackets if special characters are present,
as they indicate that we don't deal with a simple placeholder. This
simplifies the code a bit and makes defining special options slightly
easier.
Remove the flag PARSE_OPT_LITERAL_ARGHELP in the cases where the new
and more cautious handling suffices.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Placing `struct lock_file`s on the stack used to be a bad idea, because
the temp- and lockfile-machinery would keep a pointer into the struct.
But after 076aa2cbd (tempfile: auto-allocate tempfiles on heap,
2017-09-05), we can safely have lockfiles on the stack. (This applies
even if a user returns early, leaving a locked lock behind.)
Each of these `struct lock_file`s is used from within a single function.
Move them into the respective functions to make the scope clearer and
drop the staticness.
For good measure, I have inspected these sites and come to believe that
they always release the lock, with the possible exception of bailing out
using `die()` or `exit()` or by returning from a `cmd_foo()`.
As pointed out by Jeff King, it would be bad if someone held on to a
`struct lock_file *` for some reason. After some grepping, I agree with
his findings: no-one appears to be doing that.
After this commit, the remaining occurrences of "static struct
lock_file" are locks that are used from within different functions. That
is, they need to remain static. (Short of more intrusive changes like
passing around pointers to non-static locks.)
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now that the submodule-config subsystem can lazily read the gitmodules
file we no longer need to explicitly pre-read the gitmodules by calling
'gitmodules_config()' so let's remove it.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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
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>
Many commands learned to pay attention to submodule.recurse
configuration.
* sb/submodule-blanket-recursive:
builtin/fetch.c: respect 'submodule.recurse' option
builtin/push.c: respect 'submodule.recurse' option
builtin/grep.c: respect 'submodule.recurse' option
Introduce 'submodule.recurse' option for worktree manipulators
submodule loading: separate code path for .gitmodules and config overlay
reset/checkout/read-tree: unify config callback for submodule recursion
submodule test invocation: only pass additional arguments
submodule recursing: do not write a config variable twice
Any command that understands '--recurse-submodules' can have its
default changed to true, by setting the new 'submodule.recurse'
option.
This patch includes read-tree/checkout/reset for working tree
manipulating commands. Later patches will cover other commands.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The callback function is essentially duplicated 3 times. Remove all
of them and offer a new callback function, that lives in submodule.c
By putting the callback function there, we no longer need the function
'set_config_update_recurse_submodules', nor duplicate the global variable
in each builtin as well as submodule.c
In the three builtins we have different 2 ways how to load the .gitmodules
and config file, which are slightly different. git-checkout has to load
the submodule config all the time due to 23b4c7bcc5 (checkout: Use
submodule.*.ignore settings from .git/config and .gitmodules, 2010-08-28)
git-reset and git-read-tree do not respect these diff settings, so loading
the submodule configuration is optional. Also put that into submodule.c
for code deduplication.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The command line option for '--recurse-submodules' is implemented
using an OPTION_CALLBACK, which takes both the callback (that sets
the file static global variable) as well as passes the same file
static global variable to the option parsing machinery to assign it.
This is fixed in this commit by passing NULL as the variable. The
callback sets it instead
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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
"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
"git read-tree -m" requires a tree argument to name the tree to be
merged in. Git uses a cutesy error message to say so and why:
$ git read-tree -m
warning: read-tree: emptying the index with no arguments is
deprecated; use --empty
fatal: just how do you expect me to merge 0 trees?
$ git read-tree -m --empty
fatal: just how do you expect me to merge 0 trees?
When lucky, that could produce an ah-hah moment for the user, but it's
more likely to irritate and distract them.
Instead, tell the user plainly that the tree argument is
required. Also document this requirement in the git-read-tree(1)
manpage where there is room to explain it in a more straightforward way.
Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
fb1bb965 ("read-tree: deprecate syntax without tree-ish args",
2010-09-10) wanted to deprecate "git read-tree" without any tree,
which used to be the way to empty the index, and encourage use of
"git read-tree --empty" instead.
However, when used with "-m", "--empty" does not make any sense,
either, simply because merging 0 trees will result in a different
error anyway.
Omit the deprecation warning and let the code to emit real error
message diagnose the error.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert parse_tree_indirect to take a pointer to struct object_id.
Update all the callers. This transformation was achieved using the
following semantic patch and manual updates to the declaration and
definition. Update builtin/checkout.c manually as well, since it uses a
ternary expression not handled by the semantic patch.
@@
expression E1;
@@
- parse_tree_indirect(E1.hash)
+ parse_tree_indirect(&E1)
@@
expression E1;
@@
- parse_tree_indirect(E1->hash)
+ parse_tree_indirect(E1)
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a caller of parse_tree_indirect, which must be converted in
order to convert parse_object.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A new known failure mode is introduced[1], which is actually not
a failure but a feature in read-tree. Unlike checkout for which
the recursive submodule tests were originally written, read-tree does
warn about ignored untracked files that would be overwritten.
For the sake of keeping the test library for submodules generic, just
mark the test as a failure.
[1] KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
All occurrences of OPT_SET_INT were setting the value to 1;
internally OPT_BOOL is just that.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Callers of the hold_locked_index() function pass 0 when they want to
prepare to write a new version of the index file without wishing to
die or emit an error message when the request fails (e.g. somebody
else already held the lock), and pass 1 when they want the call to
die upon failure.
This option is called LOCK_DIE_ON_ERROR by the underlying lockfile
API, and the hold_locked_index() function translates the paramter to
LOCK_DIE_ON_ERROR when calling the hold_lock_file_for_update().
Replace these hardcoded '1' with LOCK_DIE_ON_ERROR and stop
translating. Callers other than the ones that are replaced with
this change pass '0' to the function; no behaviour change is
intended with this patch.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Among the callers of hold_locked_index() that passes 0:
- diff.c::refresh_index_quietly() at the end of "git diff" is an
opportunistic update; it leaks the lockfile structure but it is
just before the program exits and nobody should care.
- builtin/describe.c::cmd_describe(),
builtin/commit.c::cmd_status(),
sequencer.c::read_and_refresh_cache() are all opportunistic
updates and they are OK.
- builtin/update-index.c::cmd_update_index() takes a lock upfront
but we may end up not needing to update the index (i.e. the
entries may be fully up-to-date), in which case we do not need to
issue an error upon failure to acquire the lock. We do diagnose
and die if we indeed need to update, so it is OK.
- wt-status.c::require_clean_work_tree() IS BUGGY. It asks
silence, does not check the returned value. Compare with
callsites like cmd_describe() and cmd_status() to notice that it
is wrong to call update_index_if_able() unconditionally.
Convert struct cache_entry to use struct object_id by applying the
following semantic patch and the object_id transforms from contrib, plus
the actual change to the struct:
@@
struct cache_entry E1;
@@
- E1.sha1
+ E1.oid.hash
@@
struct cache_entry *E1;
@@
- E1->sha1
+ E1->oid.hash
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We sometimes sprintf into fixed-size buffers when we know
that the buffer is large enough to fit the input (either
because it's a constant, or because it's numeric input that
is bounded in size). Likewise with strcpy of constant
strings.
However, these sites make it hard to audit sprintf and
strcpy calls for buffer overflows, as a reader has to
cross-reference the size of the array with the input. Let's
use xsnprintf instead, which communicates to a reader that
we don't expect this to overflow (and catches the mistake in
case we do).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
-u and -i can only be given if -m, --reset, or --prefix is given.
Without parentheses, it looks like -u and -i can be used no matter
what, and the second pair of brackets is confusing.
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the interface declaration for the functions in lockfile.c from
cache.h to a new file, lockfile.h. Add #includes where necessary (and
remove some redundant includes of cache.h by files that already
include builtin.h).
Move the documentation of the lock_file state diagram from lockfile.c
to the new header file.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Correct all hits from
git grep -e '\(&&\|||\)[^ ]' -e '[^ ]\(&&\|||\)' -- '*.c'
i.e. && or || operators that are followed by anything but a SP,
or that follow something other than a SP or a HT, so that these
operators have a SP around it when necessary.
We usually refrain from making this kind of a tree-wide change in
order to avoid unnecessary conflicts with other "real work" patches,
but in this case, the end result does not have a potentially
cumbersome tree-wide impact, while this is a tree-wide cleanup.
Fixes to compat/regex/regcomp.c and xdiff/xemit.c are to replace a
HT immediately after && with a SP.
This is based on Felipe's patch to bultin/symbolic-ref.c; I did all
the finding out what other files in the whole tree need to be fixed
and did the fix and also the log message while reviewing that single
liner, so any screw-ups in this version are mine.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the type merge_fn_t to accept the array of cache_entry pointers
as const pointers to const pointers. This documents the fact that the
merge functions don't modify the cache_entry contents or replace any of
the pointers in the array.
Only a single cast is necessary in unpack_nondirectories because adding
two const modifiers at once is not allowed in C. The cast is safe in
that it doesn't mask any modfication; call_unpack_fn only needs the
array for reading.
Signed-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add const to struct cache_entry pointers throughout the tree which are
only used for reading. This allows callers to pass in const pointers.
Signed-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The option can be used to check if read-tree with the same set of other
options like "-m" and "-u" would succeed without actually changing either
the index or the working tree.
The relevant tests in the t10?? range were extended to do a read-tree -n
before the real read-tree to make sure neither the index nor any local
files were changed with -n and the same exit code as without -n is
returned. The helper functions added for that purpose reside in the new
t/lib-read-tree.sh file.
The only exception is #13 in t1004 ("unlinking an un-unlink-able
symlink"). As this is an issue of wrong directory permissions it is not
detected with -n.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allows better help text to be defined than "be verbose". Also make use
of the macro in places that already had a different description. No
object code changes intended.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently, read-tree can be run without tree-ish arguments, in which
case it will empty the index. Since this behavior is undocumented and
perhaps a bit too invasive to be the "default" action for read-tree,
deprecate it in favor of a new --empty option that does the same thing.
Signed-off-by: Jan Krüger <jk@jk.gs>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* ko/master: (2325 commits)
Git 1.7.2-rc2
backmerge a few more fixes to 1.7.1.X series
fix git branch -m in presence of cross devices
t/t0006: specify timezone as EST5 not EST to comply with POSIX
add missing && to submodule-merge testcase
t/README: document more test helpers
test-date: fix sscanf type conversion
xdiff: optimise for no whitespace difference when ignoring whitespace.
gitweb: Move evaluate_gitweb_config out of run_request
parse_date: fix signedness in timezone calculation
t0006: test timezone parsing
rerere.txt: Document forget subcommand
t/README: proposed rewording...
t/README: Document the do's and don'ts of tests
t/README: Add a section about skipping tests
t/README: Document test_expect_code
t/README: Document test_external*
t/README: Document the prereq functions, and 3-arg test_*
t/README: Typo: paralell -> parallel
t/README: The trash is in 't/trash directory.$name'
...
Conflicts:
builtin-read-tree.c
This shrinks the top-level directory a bit, and makes it much more
pleasant to use auto-completion on the thing. Instead of
[torvalds@nehalem git]$ em buil<tab>
Display all 180 possibilities? (y or n)
[torvalds@nehalem git]$ em builtin-sh
builtin-shortlog.c builtin-show-branch.c builtin-show-ref.c
builtin-shortlog.o builtin-show-branch.o builtin-show-ref.o
[torvalds@nehalem git]$ em builtin-shor<tab>
builtin-shortlog.c builtin-shortlog.o
[torvalds@nehalem git]$ em builtin-shortlog.c
you get
[torvalds@nehalem git]$ em buil<tab> [type]
builtin/ builtin.h
[torvalds@nehalem git]$ em builtin [auto-completes to]
[torvalds@nehalem git]$ em builtin/sh<tab> [type]
shortlog.c shortlog.o show-branch.c show-branch.o show-ref.c show-ref.o
[torvalds@nehalem git]$ em builtin/sho [auto-completes to]
[torvalds@nehalem git]$ em builtin/shor<tab> [type]
shortlog.c shortlog.o
[torvalds@nehalem git]$ em builtin/shortlog.c
which doesn't seem all that different, but not having that annoying
break in "Display all 180 possibilities?" is quite a relief.
NOTE! If you do this in a clean tree (no object files etc), or using an
editor that has auto-completion rules that ignores '*.o' files, you
won't see that annoying 'Display all 180 possibilities?' message - it
will just show the choices instead. I think bash has some cut-off
around 100 choices or something.
So the reason I see this is that I'm using an odd editory, and thus
don't have the rules to cut down on auto-completion. But you can
simulate that by using 'ls' instead, or something similar.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
