Skip the administrative overhead of using pthreads when only using one
thread. Instead take the non-threaded path which would be taken under
NO_PTHREADS.
The threading support was initially added in commit
5b594f457a ("Threaded grep", 2010-01-25) with a hardcoded compile-time
number of 8 threads. Later the number of threads was made configurable
in commit 89f09dd34e ("grep: add --threads=<num> option and
grep.threads configuration", 2015-12-15).
That change did not add any special handling for --threads=1. Now we
take a slightly faster path by skipping thread handling entirely when
1 thread is requested.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the pattern compilation logic under threading so that grep
doesn't compile a pattern it never ends up using on the non-threaded
code path, only to compile it again N times for N threads which will
each use their own copy, ignoring the initially compiled pattern.
This redundant compilation dates back to the initial introduction of
the threaded grep in commit 5b594f457a ("Threaded grep",
2010-01-25).
There was never any reason for doing this redundant work other than an
oversight in the initial commit. Jeff King suggested on-list in
<20170414212325.fefrl3qdjigwyitd@sigill.intra.peff.net> that this
might be needed to check the pattern for sanity before threaded
execution commences.
That's not the case. The pattern is compiled under threading in
start_threads() before any concurrent execution has started by calling
pthread_create(), so if the pattern contains an error we still do the
right thing. I.e. die with one error before any threaded execution has
commenced, instead of e.g. spewing out an error for each N threads,
which could be a regression a change like this might inadvertently
introduce.
This change is not meant as an optimization, any performance gains
from this are in the hundreds to thousands of nanoseconds at most. If
we wanted more performance here we could just re-use the compiled
patterns in multiple threads (regcomp(3) is thread-safe), or partially
re-use them and the associated structures in the case of later PCRE
JIT changes.
Rather, it's just to make the code easier to reason about. It's
confusing to debug this under threading & non-threading when the
threading codepaths redundantly compile a pattern which is never used.
The reason the patterns are recompiled is as a side-effect of
duplicating the whole grep_opt structure, which is not thread safe,
writable, and munged during execution. The grep_opt structure then
points to the grep_pat structure where pattern or patterns are stored.
I looked into e.g. splitting the API into some "do & alloc threadsafe
stuff", "spawn thread", "do and alloc non-threadsafe stuff", but the
execution time of grep_opt_dup() & pattern compilation is trivial
compared to actually executing the grep, so there was no point. Even
with the more expensive JIT changes to follow the most expensive PCRE
patterns take something like 0.0X milliseconds to compile at most[1].
The undocumented --debug mode added in commit 17bf35a3c7 ("grep: teach
--debug option to dump the parse tree", 2012-09-13) still works
properly with this change. It only emits debugging info during pattern
compilation, which is now dumped by the pattern compiled just before
the first thread is started.
1. http://sljit.sourceforge.net/pcre.html
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
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>
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>
When fopen() returns NULL, it could be because the given path does not
exist, but it could also be some other errors and the caller has to
check. Add a wrapper so we don't have to repeat the same error check
everywhere.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
copy_alternates() called fopen() without handling errors. By switching
to xfopen(), this bug is fixed.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
xfopen()
- provides error details
- explains error on reading, or writing, or whatever operation
- has l10n support
- prints file name in the error
Some of these are missing in the places that are replaced with xfopen(),
which is a clear win. In some other places, it's just less code (not as
clearly a win as the previous case but still is).
The only slight regresssion is in remote-testsvn, where we don't report
the file class (marks files) in the error messages anymore. But since
this is a _test_ svn remote transport, I'm not too concerned.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The origin, entry, and scoreboard structures are core to the blame
interface and need to be exposed for blame functionality.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Create function that populates a blame_entry and prepends it to a list.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Create function that completes setting up blame_scoreboard structure.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Create function that initializes blame_scoreboard to default values.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Either prepare_initial or prepare_final is used to determine which
commit is marked as 'final'. Call the underlying methods directly to
make this more clear.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The new method's interface is marginally cleaner than blame_sort, and
will avoid the need to expose the compare_blame_final method.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allow the interface user to decide how to handle a progress update.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allow the interface user to decide how to handle a failed sanity check,
whether that be to output with the current state or to do nothing.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The no_whole_file_rename flag is used in parts of blame that are being
moved to libgit, and should be accessible via the scoreboard structure.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The xdl_opts flags are used in parts of blame that are being moved to
libgit, and should be accessible via the scoreboard structure.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The show_root flag is used in parts of blame that are being moved to
libgit, and should be accessible via the scoreboard structure.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The reverse flag is used in parts of blame that are being moved to
libgit, and should be accessible via the scoreboard structure.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The argument from --contents is used in parts of blame that are being
moved to libgit, and should be accessible via the scoreboard structure.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Copy and move score thresholds are used in parts of blame that are being
moved to libgit, and should be accessible via the scoreboard structure.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Statistic counters are used in parts of blame that are being moved to
libgit, and should be accessible via the scoreboard structure.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Functions that will be publicly exposed should have names that better
reflect what they are a part of.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Functions that will be publicly exposed should have names that better
reflect what they are a part of.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Functions that will be publicly exposed should have names that better
reflect what they are a part of.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Functions related to blame_origin that will be publicly exposed should
have names that better reflect what they are a part of.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The scoreboard structure is core to the blame interface. Since
scoreboard will become more exposed, rename it to blame_scoreboard to
clarify what it is a part of.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The origin structure is core to the blame interface. Since origin will
become more exposed, rename it to blame_origin to clarify what it is a
part of.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
textconv_object is used in places other than blame.c and should be moved
to a more appropriate location. Other textconv related functions are
located in diff.c so that seems as good a place as any.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
With commit 21666f1 ("convert object type handling from a string to a
number", 2007-02-26), there was no longer a need for blame.c to include
blob.h but it was not removed.
Signed-off-by: Jeff Smith <whydoubt@gmail.com>
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>
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>
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>
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>
Just because the files backend can't retain reflogs for deleted
references is no reason that they shouldn't be supported by the
virtual method interface. Also, `delete_ref()` and `refs_delete_ref()`
have already gained `msg` parameters. Now let's add them to
`delete_refs()` and `refs_delete_refs()`.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* 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
...
The receive-pack program now makes sure that the push certificate
records the same set of push options used for pushing.
* jt/push-options-doc:
receive-pack: verify push options in cert
docs: correct receive.advertisePushOptions default
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
Setting "log.decorate=false" in the configuration file did not take
effect in v2.13, which has been corrected.
* ah/log-decorate-default-to-auto:
builtin/log: honor log.decorate
Earlier dddbad72 ("timestamp_t: a new data type for timestamps",
2017-04-26) updated all in-core variables, fields and function
return values that are used to store "seconds since epoch" to a new
type timestamp_t. Unfortunately one variable "cutoff", which is
used to keep track of the oldest timestamp of commit we saw on the
command line, was "long" and left behind.
Update it to timestamp_t as well.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If a commit message is edited with the "verbose" option, the buffer
will have a cut line and diff after the log message, like so:
my subject
# ------------------------ >8 ------------------------
# Do not touch the line above.
# Everything below will be removed.
diff --git a/foo.txt b/foo.txt
index 5716ca5..7601807 100644
--- a/foo.txt
+++ b/foo.txt
@@ -1 +1 @@
-bar
+baz
"git interpret-trailers" is unaware of the cut line, and assumes the
trailer block would be at the end of the whole thing. This can easily
be seen with:
$ GIT_EDITOR='git interpret-trailers --in-place --trailer Acked-by:me' \
git commit --amend -v
Teach "git interpret-trailers" to notice the cut-line and ignore the
remainder of the input when looking for a place to add new trailer
block. This makes it consistent with how "git commit -v -s" inserts a
new Signed-off-by: line.
This can be done by the same logic as the existing helper function,
wt_status_truncate_message_at_cut_line(), uses, but it wants the caller
to pass a strbuf to it. Because the function ignore_non_trailer() used
by the command takes a <pointer, length> pair, not a strbuf, steal the
logic from wt_status_truncate_message_at_cut_line() to create a new
wt_status_locate_end() helper function that takes <pointer, length>
pair, and make ignore_non_trailer() call it to help "interpret-trailers".
Since there is only one caller of wt_status_truncate_message_at_cut_line()
in cmd_commit(), rewrite it to call wt_status_locate_end() helper instead
and remove the old helper that no longer has any caller.
Signed-off-by: Brian Malehorn <bmalehorn@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some platforms have ulong that is smaller than time_t, and our
historical use of ulong for timestamp would mean they cannot
represent some timestamp that the platform allows. Invent a
separate and dedicated timestamp_t (so that we can distingiuish
timestamps and a vanilla ulongs, which along is already a good
move), and then declare uintmax_t is the type to be used as the
timestamp_t.
* js/larger-timestamps:
archive-tar: fix a sparse 'constant too large' warning
use uintmax_t for timestamps
date.c: abort if the system time cannot handle one of our timestamps
timestamp_t: a new data type for timestamps
PRItime: introduce a new "printf format" for timestamps
parse_timestamp(): specify explicitly where we parse timestamps
t0006 & t5000: skip "far in the future" test when time_t is too limited
t0006 & t5000: prepare for 64-bit timestamps
ref-filter: avoid using `unsigned long` for catch-all data type
"git clone --config var=val" is a way to populate the
per-repository configuration file of the new repository, but it did
not work well when val is an empty string. This has been fixed.
* jn/clone-add-empty-config-from-command-line:
clone: handle empty config values in -c
"git clone" learned the "--no-tags" option not to fetch all tags
initially, and also set up the tagopt not to follow any tags in
subsequent fetches.
* ab/clone-no-tags:
tests: rename a test having to do with shallow submodules
clone: add a --no-tags option to clone without tags
tests: change "cd ... && git fetch" to "cd &&\n\tgit fetch"
The colors in which "git status --short --branch" showed the names
of the current branch and its remote-tracking branch are now
configurable.
* sk/status-short-branch-color-config:
status: add color config slots for branch info in "--short --branch"
status: fix missing newline when comment chars are disabled
The codepath in "git am" that is used when running "git rebase"
leaked memory held for the log message of the commits being rebased.
* jk/am-leakfix:
am: shorten ident_split variable name in get_commit_info()
am: simplify allocations in get_commit_info()
am: fix commit buffer leak in get_commit_info()
The recent change that introduced autodecorating of refs accidentally
broke the ability of users to set log.decorate = false to override it.
When the git_log_config was traversed a second time with an option other
than log.decorate, the decoration style would be set to the automatic
style, even if the user had already overridden it. Instead of setting
the option in config parsing, set it in init_log_defaults instead.
Add a test for this case. The actual additional config option doesn't
matter, but it needs to be something not already set in the
configuration file.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Acked-by: Alex Henrie <alexhenrie24@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "--local" option instructs git-config to read or modify
the repository-level config. This doesn't make any sense if
you're not actually in a repository.
Older versions of Git would blindly try to read or write
".git/config". For reading, this would result in a quiet
failure, since there was no config to read (and thus no
matching config value). Writing would generally fail
noisily, since ".git" was unlikely to exist. But since
b1ef400ee (setup_git_env: avoid blind fall-back to ".git",
2016-10-20), we catch this in the call to git_pathdup() and
die with an assertion.
Dying is the right thing to do, but we should catch the
problem early and give a more human-friendly error message.
Note that even without --local, git-config will sometimes
default to using local repository config (e.g., when
writing). These cases are already protected by similar
checks, and covered by a test in t1308.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"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>
There has been a bug report by a corporate user that stated that
"spelling mistake of stash followed by a yes prints character 'y'
infinite times."
This analysis was false. When the spelling of a command contains
errors, the git program tries to help the user by providing candidates
which are close to the unexisting command. E.g Git prints the
following:
git: 'stahs' is not a git command. See 'git --help'.
Did you mean this?
stash
and then exits.
The problem with this hint is that it is not formally indicated as an
hint and the user is in fact encouraged to reply to the question,
whereas the Git command is already finished.
The user was unlucky enough that it was the command he was looking
for, and replied "yes" on the command line, effectively launching the
`yes` program.
The initial error is that the Git programs, when launched in
command-line mode (without interaction) must not ask questions,
because these questions would normally require a user input as a reply
that they won't handle indeed. That's a source of confusion on UX
level.
To improve the general usability of the Git suite, the following rule
was applied:
if the sentence
* appears in a non-interactive session
* is printed last before exit
* is a question addressing the user ("you")
the sentence is turned into affirmative and proposes the option.
The basic rewording of the question sentences has been extended to
other spots found in the source.
Requested at https://github.com/git/git-scm.com/issues/999 by rpai1
Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
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.
ls-files is the only caller of 'parse_pathspec()' which relies on the
behavior of the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag because it
uses the result to construct a common prefix of all provided pathspecs
which is then used to prune the index of all entries which don't have
that prefix. Since submodules entries in the index don't have a
trailing slash 'prune_cache()' will be overeager and prune a submodule
'sub' if the common prefix is 'sub/'. To correct this behavior, only
prune entries which don't match up to, but not including, a trailing
slash of the common prefix.
This is in preparation to remove the
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag in a later patch.
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>
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>
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>
In commit f6a4e61 ("push: accept push options", 2016-07-14), send-pack
was taught to include push options both within the signed cert (if the
push is a signed push) and outside the signed cert; however,
receive-pack ignores push options within the cert, only handling push
options outside the cert.
Teach receive-pack, in the case that push options are provided for a
signed push, to verify that the push options both within the cert and
outside the cert are consistent.
This sets in stone the requirement that send-pack redundantly send its
push options in 2 places, but I think that this is better than the
alternatives. Sending push options only within the cert is
backwards-incompatible with existing Git servers (which read push
options only from outside the cert), and sending push options only
outside the cert means that the push options are not signed for.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When "git checkout -m" does an in-core three-way merge to carry
local modifications forward to check out a different branch, the
code forgot to free the updated contents it has in-core.
Noticed-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This matches how the diff Porcelain works. It makes the plumbing commands
respect diff's configuration options, such as indentHeuristic, because
init_revisions() calls diff_setup() which fills in the diff_options struct.
Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If certain options like --honor-pack-keep, --local, or
--incremental are used with pack-objects, then we need to
feed each potential object to want_object_in_pack() to see
if it should be filtered out. But when the bitmap
reuse_packfile optimization is in effect, we do not call
that function at all, and in fact skip adding the objects to
the to_pack list entirely. This means we have a bug: for
certain requests we will silently ignore those options and
include objects in that pack that should not be there.
The problem has been present since the inception of the
pack-reuse code in 6b8fda2db (pack-objects: use bitmaps when
packing objects, 2013-12-21), but it was unlikely to come up
in practice. These options are generally used for on-disk
packing, not transfer packs (which go to stdout), but we've
never allowed pack reuse for non-stdout packs (until
645c432d6, we did not even use bitmaps, which the reuse
optimization relies on; after that, we explicitly turned it
off when not packing to stdout).
We can fix this by just disabling the reuse_packfile
optimization when the options are in use. In theory we could
teach the pack-reuse code to satisfy these checks, but it's
not worth the complexity. The purpose of the optimization is
to keep the amount of per-object work we do to a minimum.
But these options inherently require us to search for other
copies of each object, drowning out any benefit of the
pack-reuse optimization. But note that the optimizations
from 56dfeb626 (pack-objects: compute local/ignore_pack_keep
early, 2016-07-29) happen before pack-reuse, meaning that
specifying "--honor-pack-keep" in a repository with no .keep
files can still follow the fast path.
There are tests in t5310 that check these options with
bitmaps and --stdout, but they didn't catch the bug, and
it's hard to adapt them to do so.
One problem is that they don't use --delta-base-offset;
without that option, we always disable the reuse
optimization entirely. It would be fine to add it in (it
actually makes the test more realistic), but that still
isn't quite enough.
The other problem is that the reuse code is very picky; it
only kicks in when it can reuse most of a pack, starting
from the first byte. So we'd have to start from a fully
repacked and bitmapped state to trigger it. But the tests
for these options use a much more subtle state; they want to
be sure that the want_object_in_pack() code is allowing some
objects but not others. Doing a full repack runs counter to
that.
So this patch adds new tests at the end of the script which
create the fully-packed state and make sure that each option
is not fooled by reusable pack.
Signed-off-by: Jeff King <peff@peff.net>
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 needed to convert parse_tree_indirect.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a prerequisite to convert do_diff_cache, which is required to
convert parse_tree_indirect.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Converting checkout_fast_forward is required to convert
parse_tree_indirect.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is another caller of parse_tree_indirect.
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>
Rename this function and convert it to take a pointer to struct
object_id.
This is a prerequisite for converting get_reference, which is needed to
convert parse_object.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make the verify_pack_callback take a pointer to struct object_id.
Change the pack checksum to use GIT_MAX_RAWSZ, even though it is not
strictly an object ID. Doing so ensures resilience against future hash
size changes, and allows us to remove hard-coded assumptions about how
big the buffer needs to be.
Also, use a union to convert the pointer from nth_packed_object_sha1 to
to a pointer to struct object_id. This behavior is compatible with GCC
and clang and explicitly sanctioned by C11. The alternatives are to
just perform a cast, which would run afoul of strict aliasing rules, but
should just work, and changing the pointer into an instance of struct
object_id and copying the value. The latter operation could seriously
bloat memory usage on fsck, which already uses a lot of memory on some
repositories.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert lookup_tag to take a pointer to struct object_id.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert the lookup_tree function to take a pointer to struct object_id.
The commit was created with manual changes to tree.c, tree.h, and
object.c, plus the following semantic patch:
@@
@@
- lookup_tree(EMPTY_TREE_SHA1_BIN)
+ lookup_tree(&empty_tree_oid)
@@
expression E1;
@@
- lookup_tree(E1.hash)
+ lookup_tree(&E1)
@@
expression E1;
@@
- lookup_tree(E1->hash)
+ lookup_tree(E1)
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert lookup_blob to take a pointer to struct object_id.
The commit was created with manual changes to blob.c and blob.h, plus
the following semantic patch:
@@
expression E1;
@@
- lookup_blob(E1.hash)
+ lookup_blob(&E1)
@@
expression E1;
@@
- lookup_blob(E1->hash)
+ lookup_blob(E1)
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
All but a few callers of lookup_blob have been converted to struct
object_id. Introduce a temporary, which will be removed later, into
parse_object to ease the transition, and convert the remaining callers
so that we can update lookup_blob to take struct object_id *.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert struct delta_info and struct object_info, as well as the various
functions, to use struct object_id. Convert several hard-coded 20
values to GIT_SHA1_RAWSZ. Among the functions converted is a caller of
lookup_blob, which we will convert shortly.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert struct pack_idx_entry to use struct object_id by changing the
definition and applying the following semantic patch, plus the standard
object_id transforms:
@@
struct pack_idx_entry E1;
@@
- E1.sha1
+ E1.oid.hash
@@
struct pack_idx_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>
Convert lookup_commit, lookup_commit_or_die,
lookup_commit_reference, and lookup_commit_reference_gently to take
struct object_id arguments.
Introduce a temporary in parse_object buffer in order to convert this
function. This is required since in order to convert parse_object and
parse_object_buffer, lookup_commit_reference_gently and
lookup_commit_or_die would need to be converted. Not introducing a
temporary would therefore require that lookup_commit_or_die take a
struct object_id *, but lookup_commit would take unsigned char *,
leaving a confusing and hard-to-use interface.
parse_object_buffer will lose this temporary in a later patch.
This commit was created with manual changes to commit.c, commit.h, and
object.c, plus the following semantic patch:
@@
expression E1, E2;
@@
- lookup_commit_reference_gently(E1.hash, E2)
+ lookup_commit_reference_gently(&E1, E2)
@@
expression E1, E2;
@@
- lookup_commit_reference_gently(E1->hash, E2)
+ lookup_commit_reference_gently(E1, E2)
@@
expression E1;
@@
- lookup_commit_reference(E1.hash)
+ lookup_commit_reference(&E1)
@@
expression E1;
@@
- lookup_commit_reference(E1->hash)
+ lookup_commit_reference(E1)
@@
expression E1;
@@
- lookup_commit(E1.hash)
+ lookup_commit(&E1)
@@
expression E1;
@@
- lookup_commit(E1->hash)
+ lookup_commit(E1)
@@
expression E1, E2;
@@
- lookup_commit_or_die(E1.hash, E2)
+ lookup_commit_or_die(&E1, E2)
@@
expression E1, E2;
@@
- lookup_commit_or_die(E1->hash, E2)
+ lookup_commit_or_die(E1, E2)
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Parts of this module call lookup_commit_reference, which we want to
convert. The module is small and mostly self-contained, so convert the
rest of it while we're at it.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert register_shallow and unregister_shallow to take struct
object_id. register_shallow is a caller of lookup_commit, which we will
convert later. It doesn't make sense for the registration and
unregistration functions to have incompatible interfaces, so convert
them both.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a prerequisite to convert to lookup_commit, which we will
convert later.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adjust the callback functions to take struct object_id * instead of
unsigned char *, and modify related static functions accordingly.
Introduce a temporary object_id instance into files_reflog_expire and
copy the SHA-1 value passed in. This is necessary because the sha1
parameter can come indirectly from get_sha1. Without the temporary, it
would require much more refactoring to be able to convert this function.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The buffer allocated by shorten_unambiguous_ref() needs to be released.
Discovered by Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the `name_rev()` function is asked to dereference the tip name, it
allocates memory. But when it turns out that another tip already
described the commit better than the current one, we forgot to release
the memory.
Pointed out by Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reported by, you guessed it, Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This change addresses part of the NEEDSWORK comment above the code,
therefore the comment needs to be adjusted, too.
Discovered via Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
While POSIX states that it is okay to pass EOF to isspace() (and it seems
to be implied that EOF should *not* be treated as whitespace), and also to
pass EOF to ungetc() (which seems to be intended to fail without buffering
the character), it is much better to handle these cases explicitly. Not
only does it reduce head-scratching (and helps static analysis avoid
reporting false positives), it also lets us handle files containing
nothing but whitespace by erroring out.
Reported via Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This change plugs a couple of memory leaks and makes sure that the file
descriptor is closed in run_dir_diff().
Spotted by Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we fail to read, or parse, the file, we still want to close the file
descriptor and release the strbuf.
Reported via Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If resolve_refdup() fails it returns NULL and possibly leaves its hash
output parameter untouched. Make sure to use it only if the function
succeeded, in order to avoid accessing uninitialized memory.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If resolve_refdup() fails it returns NULL and possibly leaves its hash
output parameter untouched. Make sure to use it only if the function
succeeded, in order to avoid accessing uninitialized memory.
Found with t/t2011-checkout-invalid-head.sh --valgrind.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git clone --config" uses the following incantation to add an item to
a config file, instead of replacing an existing value:
git_config_set_multivar_gently(key, value, "^$", 0)
As long as no existing value matches the regex ^$, that works as
intended and adds to the config. When a value is empty, though, it
replaces the existing value.
Noticed while trying to set credential.helper during a clone to use a
specific helper without inheriting from ~/.gitconfig and
/etc/gitconfig. That is, I ran
git clone -c credential.helper= \
-c credential.helper=myhelper \
https://example.com/repo
intending to produce the configuration
[credential]
helper =
helper = myhelper
Without this patch, the 'helper =' line is not included and the
credential helper from /etc/gitconfig gets used.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some of the functions converted are callers of lookup_commit_reference.
However, the changes involved in converting the entire thing are not too
large, so we might as well convert it all.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This function is a caller of lookup_commit_reference_gently, which we
will convert later.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert the sole instance of unsigned char [20] to struct object_id.
cmd_prune is a caller of parse_object, which we will convert later.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert all the uses of unsigned char [20] to struct object_id. Also,
convert some hard-coded integers into constants.
name_rev_line accepts a wide variety of free-form input and only
interprets 40-character hex values, passing through everything else.
Consequently, it is not a good candidate for parse_oid_hex, which is
much stricter.
This change is a prerequisite for converting parse_object.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert the sha1 member of struct cache_tree to struct object_id by
changing the definition and applying the following semantic patch, plus
the standard object_id transforms:
@@
struct cache_tree E1;
@@
- E1.sha1
+ E1.oid.hash
@@
struct cache_tree *E1;
@@
- E1->sha1
+ E1->oid.hash
Fix up one reference to active_cache_tree which was not automatically
caught by Coccinelle. These changes are prerequisites for converting
parse_object.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The semantic patch for standard object_id transforms found two
outstanding places where we could make a transformation automatically.
Apply these changes.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a segv in 'submodule init' when url is not given for a submodule.
* jk/submodule-init-segv-fix:
submodule_init: die cleanly on submodules without url defined
Add a --no-tags option to clone without fetching any tags.
Without this change there's no easy way to clone a repository without
also fetching its tags.
When supplying --single-branch the primary remote branch will be
cloned, but in addition tags will be followed & retrieved. Now
--no-tags can be added --single-branch to clone a repository without
tags, and which only tracks a single upstream branch.
This option works without --single-branch as well, and will do a
normal clone but not fetch any tags.
Many git commands pay some fixed overhead as a function of the number
of references. E.g. creating ~40k tags in linux.git will cause a
command like `git log -1 >/dev/null` to run in over a second instead
of in a matter of milliseconds, in addition numerous other things will
slow down, e.g. "git log <TAB>" with the bash completion will slowly
show ~40k references instead of 1.
The user might want to avoid all of that overhead to simply use a
repository like that to browse the "master" branch, or something like
a CI tool might want to keep that one branch up-to-date without caring
about any other references.
Without this change the only way of accomplishing this was either by
manually tweaking the config in a fresh repository:
git init git &&
cat >git/.git/config <<EOF &&
[remote "origin"]
url = git@github.com:git/git.git
tagOpt = --no-tags
fetch = +refs/heads/master:refs/remotes/origin/master
[branch "master"]
remote = origin
merge = refs/heads/master
EOF
cd git &&
git pull
Which requires hardcoding the "master" name, which may not be the main
--single-branch would have retrieved, or alternatively by setting
tagOpt=--no-tags right after cloning & deleting any existing tags:
git clone --single-branch git@github.com:git/git.git &&
cd git &&
git config remote.origin.tagOpt --no-tags &&
git tag -l | xargs git tag -d
Which of course was also subtly buggy if --branch was pointed at a
tag, leaving the user in a detached head:
git clone --single-branch --branch v2.12.0 git@github.com:git/git.git &&
cd git &&
git config remote.origin.tagOpt --no-tags &&
git tag -l | xargs git tag -d
Now all this complexity becomes the much simpler:
git clone --single-branch --no-tags git@github.com:git/git.git
Or in the case of cloning a single tag "branch":
git clone --single-branch --branch v2.12.0 --no-tags git@github.com:git/git.git
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The building of the reflog message is using strbuf, which is not
friendly with internationalization frameworks. No other reflog
messages are translated right now and switching all the messages to
i18n would require a major rework of the way the messages are built.
Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add color config slots to be used in the status short-format when
displaying local and remote tracking branch information.
[jc: rebased on top of Peff's fix to 'git status' and tweaked the
test to check both local and remote-tracking branch output]
Signed-off-by: Stephen Kent <smkent@smkent.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The local ident_split variable is often mentioned three
times per line when dealing with its begin/end pointer
pairs. Let's use a shorter name which lets us get rid of
some long lines. Since this is a short self-contained
function, readability doesn't suffer.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After we call split_ident_line(), we have several begin/end
pairs for various parts of the ident. We then copy each into
a strbuf to create a single string, and then detach that
string. We can instead skip the strbuf entirely and just
duplicate the strings directly.
This is shorter, and it makes it more obvious that we are
not leaking the strbuf (we were not before, because every
code path either died or hit a strbuf_detach).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Calling logmsg_reencode() may allocate a buffer for the
commit message (because we need to load it from disk, or
because it needs re-encoded). We must "unuse" it afterwards
to free it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Git's source code assumes that unsigned long is at least as precise as
time_t. Which is incorrect, and causes a lot of problems, in particular
where unsigned long is only 32-bit (notably on Windows, even in 64-bit
versions).
So let's just use a more appropriate data type instead. In preparation
for this, we introduce the new `timestamp_t` data type.
By necessity, this is a very, very large patch, as it has to replace all
timestamps' data type in one go.
As we will use a data type that is not necessarily identical to `time_t`,
we need to be very careful to use `time_t` whenever we interact with the
system functions, and `timestamp_t` everywhere else.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We already do so for --window=<n> and --depth=<n>; this will help
when the user wants to force --threads=1 for reproducible testing
without getting affected by racing multiple threads.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allow to lock a worktree immediately after it's created. This helps
prevent a race between "git worktree add; git worktree lock" and
"git worktree prune".
* nd/worktree-add-lock:
worktree add: add --lock option
While handy, "git_path()" is a dangerous function to use as a
callsite that uses it safely one day can be broken by changes
to other code that calls it. Reduction of its use continues.
* jk/war-on-git-path:
am: drop "dir" parameter from am_state_init
replace strbuf_addstr(git_path()) with git_path_buf()
replace xstrdup(git_path(...)) with git_pathdup(...)
use git_path_* helper functions
branch: add edit_description() helper
bisect: add git_path_bisect_terms helper
The recently introduced conditional inclusion of configuration did
not work well when early-config mechanism was involved.
* nd/conditional-config-in-early-config:
config: correct file reading order in read_early_config()
config: handle conditional include when $GIT_DIR is not set up
config: prepare to pass more info in git_config_with_options()
"git rebase" learns "--signoff" option.
* gb/rebase-signoff:
rebase: pass --[no-]signoff option to git am
builtin/am: fold am_signoff() into am_append_signoff()
builtin/am: honor --signoff also when --rebasing
When we init a submodule, we try to die when it has no URL
defined:
url = xstrdup(sub->url);
if (!url)
die(...);
But that's clearly nonsense. xstrdup() will never return
NULL, and if sub->url is NULL, we'll segfault.
These two bits of code need to be flipped, so we check
sub->url before looking at it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
gethostname(2) may not NUL terminate the buffer if hostname does
not fit; unfortunately there is no easy way to see if our buffer
was too small, but at least this will make sure we will not end up
using garbage past the end of the buffer.
* dt/xgethostname-nul-termination:
xgethostname: handle long hostnames
use HOST_NAME_MAX to size buffers for gethostname(2)
"ls-files --recurse-submodules" did not quite work well in a
project with nested submodules.
* jk/ls-files-recurse-submodules-fix:
ls-files: fix path used when recursing into submodules
ls-files: fix recurse-submodules with nested submodules
Various small fixes.
* rs/misc-cppcheck-fixes:
server-info: avoid calling fclose(3) twice in update_info_file()
files_for_each_reflog_ent_reverse(): close stream and free strbuf on error
am: close stream on error, but not stdin
Add finishing touches to a recent topic.
* jk/quarantine-received-objects:
refs: reject ref updates while GIT_QUARANTINE_PATH is set
receive-pack: document user-visible quarantine effects
receive-pack: drop tmp_objdir_env from run_update_hook
The index file has a trailing SHA-1 checksum to detect file
corruption, and historically we checked it every time the index
file is used. Omit the validation during normal use, and instead
verify only in "git fsck".
* jh/verify-index-checksum-only-in-fsck:
read-cache: force_verify_index_checksum
$GIT_DIR may in some cases be normalized with all symlinks resolved
while "gitdir" path expansion in the pattern does not receive the
same treatment, leading to incorrect mismatch. This has been fixed.
* nd/conditional-config-include:
config: resolve symlinks in conditional include's patterns
path.c: and an option to call real_path() in expand_user_path()
Currently, Git's source code treats all timestamps as if they were
unsigned longs. Therefore, it is okay to write "%lu" when printing them.
There is a substantial problem with that, though: at least on Windows,
time_t is *larger* than unsigned long, and hence we will want to switch
away from the ill-specified `unsigned long` data type.
So let's introduce the pseudo format "PRItime" (currently simply being
defined to "lu") to make it easier to change the data type used for
timestamps.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently, Git's source code represents all timestamps as `unsigned
long`. In preparation for using a more appropriate data type, let's
introduce a symbol `parse_timestamp` (currently being defined to
`strtoul`) where appropriate, so that we can later easily switch to,
say, use `strtoull()` instead.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-reset is yet another working tree manipulator, which should
be taught about submodules.
When a user uses git-reset and requests to recurse into submodules,
this will reset the submodules to the object name as recorded in the
superproject, detaching the HEADs.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The only caller of this function passes in a static buffer
returned from git_path(). This looks dangerous at first
glance, but turns out to be OK because the first thing we do
is xstrdup() the result.
Let's turn this into a git_pathdup(). That's slightly more
efficient (no extra copy), and makes it easier to audit for
dangerous git_path() invocations.
Since there's only a single caller, let's just set this
default path inside the init function. That makes the memory
ownership clear.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Writing directly into the strbuf avoids a useless copy of
the data, and dropping calls to git_path() makes it easier
to audit for dangerous calls.
Note that git_path() does an implicit strbuf_reset(), but in
each of these cases we were either already doing that reset,
or writing into a fresh strbuf anyway.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It's more efficient to use git_pathdup(), as it skips an
extra copy of the path. And by removing some calls to
git_path(), it makes it easier to audit for dangerous uses.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Long ago we added functions like git_path_merge_msg() to
replace the more dangerous git_path("MERGE_MSG"). Over time
some new calls to the latter have crept it. Let's convert
them to use the safer form.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rather than have a variable with a short name that is fed to
git_path(), let's add a helper function that returns the
full path. This avoids the dangerous git_path() function.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As explained in the document. This option has an advantage over the
command sequence "git worktree add && git worktree lock": there will be
no gap that somebody can accidentally "prune" the new worktree (or soon,
explicitly "worktree remove" it).
"worktree add" does keep a lock on while it's preparing the worktree.
If --lock is specified, this lock remains after the worktree is created.
Suggested-by: David Taylor <David.Taylor@dell.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Call clear_pathspec() to release resources immediately before the
cmd_grep() function returns.
* ab/grep-plug-pathspec-leak:
grep: plug a trivial memory leak
The "submodule" specific field in the ref_store structure is
replaced with a more generic "gitdir" that can later be used also
when dealing with ref_store that represents the set of refs visible
from the other worktrees.
* nd/files-backend-git-dir: (28 commits)
refs.h: add a note about sorting order of for_each_ref_*
t1406: new tests for submodule ref store
t1405: some basic tests on main ref store
t/helper: add test-ref-store to test ref-store functions
refs: delete pack_refs() in favor of refs_pack_refs()
files-backend: avoid ref api targeting main ref store
refs: new transaction related ref-store api
refs: add new ref-store api
refs: rename get_ref_store() to get_submodule_ref_store() and make it public
files-backend: replace submodule_allowed check in files_downcast()
refs: move submodule code out of files-backend.c
path.c: move some code out of strbuf_git_path_submodule()
refs.c: make get_main_ref_store() public and use it
refs.c: kill register_ref_store(), add register_submodule_ref_store()
refs.c: flatten get_ref_store() a bit
refs: rename lookup_ref_store() to lookup_submodule_ref_store()
refs.c: introduce get_main_ref_store()
files-backend: remove the use of git_path()
files-backend: add and use files_ref_path()
files-backend: add and use files_reflog_path()
...
"git push --recurse-submodules --push-option=<string>" learned to
propagate the push option recursively down to pushes in submodules.
* bw/push-options-recursively-to-submodules:
push: propagate remote and refspec with --recurse-submodules
submodule--helper: add push-check subcommand
remote: expose parse_push_refspec function
push: propagate push-options with --recurse-submodules
push: unmark a local variable as static
Conversion from unsigned char [40] to struct object_id continues.
* bc/object-id:
Documentation: update and rename api-sha1-array.txt
Rename sha1_array to oid_array
Convert sha1_array_for_each_unique and for_each_abbrev to object_id
Convert sha1_array_lookup to take struct object_id
Convert remaining callers of sha1_array_lookup to object_id
Make sha1_array_append take a struct object_id *
sha1-array: convert internal storage for struct sha1_array to object_id
builtin/pull: convert to struct object_id
submodule: convert check_for_new_submodule_commits to object_id
sha1_name: convert disambiguate_hint_fn to take object_id
sha1_name: convert struct disambiguate_state to object_id
test-sha1-array: convert most code to struct object_id
parse-options-cb: convert sha1_array_append caller to struct object_id
fsck: convert init_skiplist to struct object_id
builtin/receive-pack: convert portions to struct object_id
builtin/pull: convert portions to struct object_id
builtin/diff: convert to struct object_id
Convert GIT_SHA1_RAWSZ used for allocation to GIT_MAX_RAWSZ
Convert GIT_SHA1_HEXSZ used for allocation to GIT_MAX_HEXSZ
Define new hash-size constants for allocating memory
If the full hostname doesn't fit in the buffer supplied to
gethostname, POSIX does not specify whether the buffer will be
null-terminated, so to be safe, we should do it ourselves. Introduce
new function, xgethostname, which ensures that there is always a \0
at the end of the buffer.
Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
POSIX limits the length of host names to HOST_NAME_MAX. Export the
fallback definition from daemon.c and use this constant to make all
buffers used with gethostname(2) big enough for any possible result
and a terminating NUL.
Inspired-by: David Turner <dturner@twosigma.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: David Turner <dturner@twosigma.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Don't assume that the current working directory is the root of the
repository. Correctly generate the path for the recursing child
processes by building it from the work_tree() root instead. Otherwise if
we run ls-files using --git-dir or --work-tree it will not work
correctly as it attempts to change directory into a potentially invalid
location. Best case, it doesn't exist and we produce an error. Worst
case we cd into the wrong location and unknown behavior occurs.
Add a new test which highlights this possibility.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Recent update to for_each_replace_name() to make it use a strbuf in
place of a fixed buffer forgot to release the memory held by the
strbuf before leaving the function.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
So far we can only pass one flag, respect_includes, to thie function. We
need to pass some more (non-flag even), so let's make it accept a struct
instead of an integer.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since commit e77aa336f1 ("ls-files: optionally recurse into
submodules", 2016-10-07) ls-files has known how to recurse into
submodules when displaying files.
Unfortunately this fails for certain cases, including when nesting more
than one submodule, called from within a submodule that itself has
submodules, or when the GIT_DIR environemnt variable is set.
Prior to commit b58a68c1c1 ("setup: allow for prefix to be passed to
git commands", 2017-03-17) this resulted in an error indicating that
--prefix and --super-prefix were incompatible.
After this commit, instead, the process loops forever with a GIT_DIR set
to the parent and continuously reads the parent submodule files and
recursing forever.
Fix this by preparing the environment properly for submodules when
setting up the child process. This is similar to how other commands such
as grep behave.
This was not caught by the original tests because the scenario is
avoided if the submodules are created separately and not stored as the
standard method of putting the submodule git directory under
.git/modules/<name>. We can update the test to show the failure by the
addition of "git submodule absorbgitdirs" to the test case. However,
note that this new test would run forever without the necessary fix in
this patch.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update error handling for codepath that deals with corrupt loose
objects.
* jk/loose-object-info-report-error:
index-pack: detect local corruption in collision check
sha1_loose_object_info: return error for corrupted objects
Code clean-up.
* jk/snprintf-cleanups:
daemon: use an argv_array to exec children
gc: replace local buffer with git_path
transport-helper: replace checked snprintf with xsnprintf
convert unchecked snprintf into xsnprintf
combine-diff: replace malloc/snprintf with xstrfmt
replace unchecked snprintf calls with heap buffers
receive-pack: print --pack-header directly into argv array
name-rev: replace static buffer with strbuf
create_branch: use xstrfmt for reflog message
create_branch: move msg setup closer to point of use
avoid using mksnpath for refs
avoid using fixed PATH_MAX buffers for refs
fetch: use heap buffer to format reflog
tag: use strbuf to format tag header
diff: avoid fixed-size buffer for patch-ids
odb_mkstemp: use git_path_buf
odb_mkstemp: write filename into strbuf
do not check odb_mkstemp return value for errors
Avoid closing stdin, but do close an actual input file on error exit.
Found with Cppcheck.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are no more direct calls to am_signoff(), so we can fold its
logic in am_append_signoff().
(This is done in a separate commit rather than in the previous one, to
make it easier to revert this specific change if additional calls are
ever introduced.)
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signoff is handled in parse_mail(), but not in parse_mail_rebasing(),
since the latter is only used when git-rebase calls git-am with the
--rebasing option, and --signoff is never passed in this case.
In order to introduce (in the upcoming commits) support for
`git-rebase --signoff`, we must make git-am pay attention to it also
in the rebase case. This can be done by moving the conditional
addition of the signoff from parse_mail() to the caller am_run(),
after either of the parse_mail*() functions were called.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since 722ff7f87 (receive-pack: quarantine objects until
pre-receive accepts, 2016-10-03), we have to feed the
pre-receive hook the tmp_objdir environment, so that git
programs run from the hook know where to find the objects.
That commit modified run_update_hook() to do the same, but
there it is a noop. By the time we get to the update hooks,
we have already migrated the objects from quarantine, and so
tmp_objdir_env() will always return NULL. We can drop this
useless call.
Note that the ordering here and the lack of support for the
update hook is intentional. The update hook calls are
interspersed with actual ref updates, and we must migrate
the objects before any refs are updated (since otherwise
those refs would appear broken to outside processes). So the
only other options are:
- remain in quarantine for the _first_ ref, but not the
others. This is sufficiently confusing that it can be
rejected outright.
- run all the individual update hooks first, then migrate,
then update all the refs. But this changes the repository
state that the update hooks see (i.e., whether or not
refs from the same push are updated yet or not).
So the functionality is fine and remains unchanged with this
patch; we're just cleaning up a useless and confusing line
of code.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the cleanup phase for the grep command to free the pathspec
struct that's allocated earlier in the same block, and used just a few
lines earlier.
With "grep hi README.md" valgrind reports a loss of 239 bytes now,
down from 351.
The relevant --num-callers=40 --leak-check=full --show-leak-kinds=all
backtrace is:
[...] 187 (112 direct, 75 indirect) bytes in 1 blocks are definitely lost in loss record 70 of 110
[...] at 0x4C2BBAF: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
[...] by 0x60B339: do_xmalloc (wrapper.c:59)
[...] by 0x60B2F6: xmalloc (wrapper.c:86)
[...] by 0x576B37: parse_pathspec (pathspec.c:652)
[...] by 0x4519F0: cmd_grep (grep.c:1215)
[...] by 0x4062EF: run_builtin (git.c:371)
[...] by 0x40544D: handle_builtin (git.c:572)
[...] by 0x4060A2: run_argv (git.c:624)
[...] by 0x4051C6: cmd_main (git.c:701)
[...] by 0x4C5901: main (common-main.c:43)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Acked-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teach git to skip verification of the SHA1-1 checksum at the end of
the index file in verify_hdr() which is called from read_index()
unless the "force_verify_index_checksum" global variable is set.
Teach fsck to force this verification.
The checksum verification is for detecting disk corruption, and for
small projects, the time it takes to compute SHA-1 is not that
significant, but for gigantic repositories this calculation adds
significant time to every command.
These effect can be seen using t/perf/p0002-read-cache.sh:
Test HEAD~1 HEAD
--------------------------------------------------------------------------------------
0002.1: read_cache/discard_cache 1000 times 0.66(0.44+0.20) 0.30(0.27+0.02) -54.5%
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the next patch we need the ability to expand '~' to
real_path($HOME). But we can't do that from outside because '~' is part
of a pattern, not a true path. Add an option to expand_user_path() to do
so.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It only has one caller, not worth keeping just for convenience.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The left and right base directories were pointed to the buf field of
two strbufs, which were subject to change.
A contrived test case shows the problem where a file with a long enough
name to force the strbuf to grow is up-to-date (hence the code path is
used where the work tree's version of the file is reused), and then a
file that is not up-to-date needs to be written (hence the code path is
used where checkout_entry() uses the previously recorded base_dir that
is invalid by now).
Let's just copy the base_dir strings for use with checkout_entry(),
never touch them until the end, and release them then. This is an easily
verifiable fix (as opposed to the next-obvious alternative: to re-set
base_dir after every loop iteration).
This fixes https://github.com/git-for-windows/git/issues/1124
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add the 'push-check' subcommand to submodule--helper which is used to
check if the provided remote and refspec can be used as part of a push
operation in the submodule.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The default behaviour of "git log" in an interactive session has
been changed to enable "--decorate".
* ah/log-decorate-default-to-auto:
log: if --decorate is not given, default to --decorate=auto
"git tag/branch/for-each-ref" family of commands long allowed to
filter the refs by "--contains X" (show only the refs that are
descendants of X), "--merged X" (show only the refs that are
ancestors of X), "--no-merged X" (show only the refs that are not
ancestors of X). One curious omission, "--no-contains X" (show
only the refs that are not descendants of X) has been added to
them.
* ab/ref-filter-no-contains:
tag: add tests for --with and --without
ref-filter: reflow recently changed branch/tag/for-each-ref docs
ref-filter: add --no-contains option to tag/branch/for-each-ref
tag: change --point-at to default to HEAD
tag: implicitly supply --list given another list-like option
tag: change misleading --list <pattern> documentation
parse-options: add OPT_NONEG to the "contains" option
tag: add more incompatibles mode tests
for-each-ref: partly change <object> to <commit> in help
tag tests: fix a typo in a test description
tag: remove a TODO item from the test suite
ref-filter: add test for --contains on a non-commit
ref-filter: make combining --merged & --no-merged an error
tag doc: reword --[no-]merged to talk about commits, not tips
tag doc: split up the --[no-]merged documentation
tag doc: move the description of --[no-]merged earlier
There isn't any obvious reason for the 'struct string_list push_options'
and 'struct string_list_item *item' to be marked as static, so unmark
them as being static. Also, clear the push_options string_list to
prevent memory leaking.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we notice that we have a local copy of an incoming
object, we compare the two objects to make sure we haven't
found a collision. Before we get to the actual object
bytes, though, we compare the type and size from
sha1_object_info().
If our local object is corrupted, then the type will be
OBJ_BAD, which obviously will not match the incoming type,
and we'll report "SHA1 COLLISION FOUND" (with capital
letters and everything). This is confusing, as the problem
is not a collision but rather local corruption. We should
report that instead (just like we do if reading the rest of
the object content fails a few lines later).
Note that we _could_ just ignore the error and mark it as a
non-collision. That would let you "git fetch" to replace a
corrupted object. But it's not a very reliable method for
repairing a repository. The earlier want/have negotiation
tries to get the other side to omit objects we already have,
and it would not realize that we are "missing" this
corrupted object. So we're better off complaining loudly
when we see corruption, and letting the user take more
drastic measures to repair (like making a full clone
elsewhere and copying the pack into place).
Note that the test sets transfer.unpackLimit in the
receiving repository so that we use index-pack (which is
what does the collision check). Normally for such a small
push we'd use unpack-objects, which would simply try to
write the loose object, and discard the new one when we see
that there's already an old one.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since this structure handles an array of object IDs, rename it to struct
oid_array. Also rename the accessor functions and the initialization
constant.
This commit was produced mechanically by providing non-Documentation
files to the following Perl one-liners:
perl -pi -E 's/struct sha1_array/struct oid_array/g'
perl -pi -E 's/\bsha1_array_/oid_array_/g'
perl -pi -E 's/SHA1_ARRAY_INIT/OID_ARRAY_INIT/g'
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make sha1_array_for_each_unique take a callback using struct object_id.
Since one of these callbacks is an argument to for_each_abbrev, convert
those as well. Rename various functions, replacing "sha1" with "oid".
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert this function by changing the declaration and definition and
applying the following semantic patch to update the callers:
@@
expression E1, E2;
@@
- sha1_array_lookup(E1, E2.hash)
+ sha1_array_lookup(E1, &E2)
@@
expression E1, E2;
@@
- sha1_array_lookup(E1, E2->hash)
+ sha1_array_lookup(E1, E2)
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are a very small number of callers which don't already use struct
object_id. Convert them.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert the callers to pass struct object_id by changing the function
declaration and definition and applying the following semantic patch:
@@
expression E1, E2;
@@
- sha1_array_append(E1, E2.hash)
+ sha1_array_append(E1, &E2)
@@
expression E1, E2;
@@
- sha1_array_append(E1, E2->hash)
+ sha1_array_append(E1, E2)
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We probe the "17/" loose object directory for auto-gc, and
use a local buffer to format the path. We can just use
git_path() for this. It handles paths of any length
(reducing our error handling). And because we feed the
result straight to a system call, we can just use the static
variant.
Note that git_path also knows the string "objects/" is
special, and will replace it with git_object_directory()
when necessary.
Another alternative would be to use sha1_file_name() for the
pretend object "170000...", but that ends up being more
hassle for no gain, as we have to truncate the final path
component.
Signed-off-by: Jeff King <peff@peff.net>
We'd prefer to avoid unchecked snprintf calls because
truncation can lead to unexpected results.
These are all cases where truncation shouldn't ever happen,
because the input to snprintf is fixed in size. That makes
them candidates for xsnprintf(), but it's simpler still to
just use the heap, and then nobody has to wonder if "100" is
big enough.
We'll use xstrfmt() where possible, and a strbuf when we need
the resulting size or to reuse the same buffer in a loop.
Signed-off-by: Jeff King <peff@peff.net>
After receive-pack reads the pack header from the client, it
feeds the already-read part to index-pack and unpack-objects
via their --pack-header command-line options. To do so, we
format it into a fixed buffer, then duplicate it into the
child's argv_array.
Our buffer is long enough to handle any possible input, so
this isn't wrong. But it's more complicated than it needs to
be; we can just argv_array_pushf() the final value and avoid
the intermediate copy. This drops the magic number and is
more efficient, too.
Note that we need to push to the argv_array in order, which
means we can't do the push until we are in the "unpack-objects
versus index-pack" conditional. Rather than duplicate the
slightly complicated format specifier, I pushed it into a
helper function.
Signed-off-by: Jeff King <peff@peff.net>
When name-rev needs to format an actual name, we do so into
a fixed-size buffer. That includes the actual ref tip, as
well as any traversal information. Since refs can exceed
1024 bytes, this means you can get a bogus result. E.g.,
doing:
git tag $(perl -e 'print join("/", 1..1024)')
git describe --contains HEAD^
results in ".../282/283", when it should be
".../1023/1024~1".
We can solve this by using a heap buffer. We'll use a
strbuf, which lets us write into the same buffer from our
loop without having to reallocate.
Signed-off-by: Jeff King <peff@peff.net>
Many functions which handle refs use a PATH_MAX-sized buffer
to do so. This is mostly reasonable as we have to write
loose refs into the filesystem, and at least on Linux the 4K
PATH_MAX is big enough that nobody would care. But:
1. The static PATH_MAX is not always the filesystem limit.
2. On other platforms, PATH_MAX may be much smaller.
3. As we move to alternate ref storage, we won't be bound
by filesystem limits.
Let's convert these to heap buffers so we don't have to
worry about truncation or size limits.
We may want to eventually constrain ref lengths for sanity
and to prevent malicious names, but we should do so
consistently across all platforms, and in a central place
(like the ref code).
Signed-off-by: Jeff King <peff@peff.net>
Part of the reflog content comes from the environment, which
can be much larger than our fixed buffer. Let's use a heap
buffer so we avoid truncating it.
Signed-off-by: Jeff King <peff@peff.net>
We format the tag header into a fixed 1024-byte buffer. But
since the tag-name and tagger ident can be arbitrarily
large, we may unceremoniously die with "tag header too big".
Let's just use a strbuf instead.
Note that it looks at first glance like we can just format
this directly into the "buf" strbuf where it will ultimately
go. But that buffer may already contain the tag message, and
we have no easy way to prepend formatted data to a strbuf
(we can only splice in an already-generated buffer). This
isn't a performance-critical path, so going through an extra
buffer isn't a big deal.
Signed-off-by: Jeff King <peff@peff.net>
"git receive-pack" could have been forced to die by attempting
allocate an unreasonably large amount of memory with a crafted push
certificate; this has been fixed.
* bc/push-cert-receive-fix:
builtin/receive-pack: fix incorrect pointer arithmetic
Some debugging output from "git describe" were marked for l10n,
but some weren't. Mark missing ones for l10n.
* mg/describe-debug-l10n:
l10n: de: translate describe debug terms
describe: localize debug output fully
A few commands that recently learned the "--recurse-submodule"
option misbehaved when started from a subdirectory of the
superproject.
* bw/recurse-submodules-relative-fix:
ls-files: fix bug when recursing with relative pathspec
ls-files: fix typo in variable name
grep: fix bug when recursing with relative pathspec
setup: allow for prefix to be passed to git commands
grep: fix help text typo
"what URL do we want to update this submodule?" and "are we
interested in this submodule?" are split into two distinct
concepts, and then the way used to express the latter got extended,
paving a way to make it easier to manage a project with many
submodules and make it possible to later extend use of multiple
worktrees for a project with submodules.
* bw/submodule-is-active:
submodule add: respect submodule.active and submodule.<name>.active
submodule--helper init: set submodule.<name>.active
clone: teach --recurse-submodules to optionally take a pathspec
submodule init: initialize active submodules
submodule: decouple url and submodule interest
submodule--helper clone: check for configured submodules using helper
submodule sync: use submodule--helper is-active
submodule sync: skip work for inactive submodules
submodule status: use submodule--helper is-active
submodule--helper: add is-active subcommand
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
* jc/merge-drop-old-syntax:
merge: drop 'git merge <message> HEAD <commit>' syntax
In order to checkout files, difftool reads "diff --raw"
output and feeds the names to checkout_entry(). That
function requires us to have a "struct cache_entry". And
because that struct uses a FLEX_ARRAY for the name field, we
have to actually copy in our new name.
The current code allocates a single re-usable cache_entry
that can hold a name up to PATH_MAX, and then copies
filenames into it using strcpy(). But there's no guarantee
that incoming names are smaller than PATH_MAX. They've come
from "diff --raw" output which might be diffing between two
trees (and hence we'd be subject to the PATH_MAX of some
other system, or even none at all if they were created
directly via "update-index").
We can fix this by using make_cache_entry() to create a
correctly-sized cache_entry for each name. This incurs an
extra allocation per file, but this is negligible compared
to actually writing out the file contents.
To make this simpler, we can push this procedure into a new
helper function. Note that we can also get rid of the "len"
variables for src_path and dst_path (and in fact we must, as
the compiler complains that they are unused).
Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As xgetcwd() returns an allocated buffer, we should free this
buffer when we don't need it any more.
This was found by Coverity.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git name-rev" assigned a phony "far in the future" date to tips of
refs that are not pointing at tag objects, and favored names based
on a ref with the oldest date. This made it almost impossible for
an unannotated tags and branches to be counted as a viable base,
which was especially problematic when the command is run with the
"--tags" option. If an unannotated tag that points at an ancient
commit and an annotated tag that points at a much newer commit
reaches the commit that is being named, the old unannotated tag was
ignored.
Update the "taggerdate" field of the rev-name structure, which is
initialized from the tip of ref, to have the committer date if the
object at the tip of ref is a commit, not a tag, so that we can
optionally take it into account when doing "is this name better?"
comparison logic.
When "name-rev" is run without the "--tags" option, the general
expectation is still to name the commit based on a tag if possible,
but use non-tag refs as fallback, and tiebreak among these non-tag
refs by favoring names with shorter hops from the tip. The use of a
phony "far in the future" date in the original code was an effective
way to ensure this expectation is held: a non-tag tip gets the same
"far in the future" timestamp, giving precedence to tags, and among
non-tag tips, names with shorter hops are preferred over longer
hops, without taking the "taggerdate" into account. As we are
taking over the "taggerdate" field to store the committer date for
tips with commits:
(1) keep the original logic when comparing names based on two refs
both of which are from refs/tags/;
(2) favoring a name based on a ref in refs/tags/ hierarchy over
a ref outside the hierarchy;
(3) between two names based on a ref both outside refs/tags/, give
precedence to a name with shorter hops and use "taggerdate"
only to tie-break.
A change to t4202 is a natural consequence. The test creates a
commit on a branch "side" and points at it with an unannotated tag
"refs/tags/side-2". The original code couldn't decide which one to
favor at all, and gave a name based on a branch (simply because
refs/heads/side sorts earlier than refs/tags/side-2). Because the
updated logic is taught to favor refs in refs/tags/ hierarchy, the
the test is updated to expect to see tags/side-2 instead.
[mjg: open-coded the comparisons in is_better_name(), dropping a
helper macro used in the original]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael J Gruber <git@grubix.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we encounter a new ref that could describe the commit we are
looking at, we compare the name that is formed using that ref and
the name we found so far and pick a better one.
Factor the comparison logic out to a separate helper function, while
keeping the current logic the same (i.e. a name that is based on an
older tag is better, and if two tags of the same age can reach the
commit, the one with fewer number of hops to reach the commit is
better).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael J Gruber <git@grubix.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The odb_mkstemp() function expects the caller to provide a
fixed buffer to write the resulting tempfile name into. But
it creates the template using snprintf without checking the
return value. This means we could silently truncate the
filename.
In practice, it's unlikely that the truncation would end in
the template-pattern that mkstemp needs to open the file. So
we'd probably end up failing either way, unless the path was
specially crafted.
The simplest fix would be to notice the truncation and die.
However, we can observe that most callers immediately
xstrdup() the result anyway. So instead, let's switch to
using a strbuf, which is easier for them (and isn't a big
deal for the other 2 callers, who can just strbuf_release
when they're done with it).
Note that many of the callers used static buffers, but this
was purely to avoid putting a large buffer on the stack. We
never passed the static buffers out of the function, so
there's no complicated memory handling we need to change.
Signed-off-by: Jeff King <peff@peff.net>
The odb_mkstemp function does not return an error; it dies
on failure instead. But many of its callers compare the
resulting descriptor against -1 and die themselves.
Mostly this is just pointless, but it does raise a question
when looking at the callers: if they show the results of the
"template" buffer after a failure, what's in it? The answer
is: it doesn't matter, because it cannot happen.
So let's make that clear by removing the bogus error checks.
In bitmap_writer_finish(), we can drop the error-handling
code entirely. In the other two cases, it's shared with the
open() in another code path; we can just move the
error-check next to that open() call.
And while we're at it, let's flesh out the function's
docstring a bit to make the error behavior clear.
Signed-off-by: Jeff King <peff@peff.net>
Code clean-up.
* jk/fast-import-cleanup:
pack.h: define largest possible encoded object size
encode_in_pack_object_header: respect output buffer length
fast-import: use xsnprintf for formatting headers
fast-import: use xsnprintf for writing sha1s
"git checkout" is taught the "--recurse-submodules" option.
* sb/checkout-recurse-submodules:
builtin/read-tree: add --recurse-submodules switch
builtin/checkout: add --recurse-submodules switch
entry.c: create submodules when interesting
unpack-trees: check if we can perform the operation for submodules
unpack-trees: pass old oid to verify_clean_submodule
update submodules: add submodule_move_head
submodule.c: get_super_prefix_or_empty
update submodules: move up prepare_submodule_repo_env
submodules: introduce check to see whether to touch a submodule
update submodules: add a config option to determine if submodules are updated
update submodules: add submodule config parsing
make is_submodule_populated gently
lib-submodule-update.sh: define tests for recursing into submodules
lib-submodule-update.sh: replace sha1 by hash
lib-submodule-update: teach test_submodule_content the -C <dir> flag
lib-submodule-update.sh: do not use ./. as submodule remote
lib-submodule-update.sh: reorder create_lib_submodule_repo
submodule--helper.c: remove duplicate code
connect_work_tree_and_git_dir: safely create leading directories
Code clean-up.
* jk/pack-name-cleanups:
index-pack: make pointer-alias fallbacks safer
replace snprintf with odb_pack_name()
odb_pack_keep(): stop generating keepfile name
sha1_file.c: make pack-name helper globally accessible
move odb_* declarations out of git-compat-util.h
"git fetch" that requests a commit by object name, when the other
side does not allow such an request, failed without much
explanation.
* mm/fetch-show-error-message-on-unadvertised-object:
fetch-pack: add specific error for fetching an unadvertised object
fetch_refs_via_pack: call report_unmatched_refs
fetch-pack: move code to report unmatched refs to a function
"git branch @" created refs/heads/@ as a branch, and in general the
code that handled @{-1} and @{upstream} was a bit too loose in
disambiguating.
* jk/interpret-branch-name:
checkout: restrict @-expansions when finding branch
strbuf_check_ref_format(): expand only local branches
branch: restrict @-expansions when deleting
t3204: test git-branch @-expansion corner cases
interpret_branch_name: allow callers to restrict expansions
strbuf_branchname: add docstring
strbuf_branchname: drop return value
interpret_branch_name: move docstring to header file
interpret_branch_name(): handle auto-namelen for @{-1}
"git repack --depth=<n>" for a long time busted the specified depth
when reusing delta from existing packs. This has been corrected.
* jk/delta-chain-limit:
pack-objects: convert recursion to iteration in break_delta_chain()
pack-objects: enforce --depth limit in reused deltas
Make the internal storage for struct sha1_array use an array of struct
object_id internally. Update the users of this struct which inspect its
internals.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert virtually all uses of unsigned char [20] to struct object_id.
Leave all the arguments that come from struct sha1_array, as these will
be converted in a later patch.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
All of the callers of this function have been converted, so convert this
function and update the callers. This function also calls
sha1_array_append, which we'll convert shortly.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert some hardcoded constants into uses of parse_oid_hex.
Additionally, convert all uses of struct command, and miscellaneous
other functions necessary for that. This work is necessary to be able
to convert sha1_array_append later on.
To avoid needing to specify a constant, reject shallow lines with the
wrong length instead of simply ignoring them.
Note that in queue_command we are guaranteed to have a NUL-terminated
buffer or at least one byte of overflow that we can safely read, so the
linelen check can be elided. We would die in such a case, but not read
invalid memory.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If we had already processed the last newline in a push certificate, we
would end up subtracting NULL from the end-of-certificate pointer when
computing the length of the line. This would have resulted in an
absurdly large length, and possibly a buffer overflow. Instead,
subtract the beginning-of-certificate pointer from the
end-of-certificate pointer, which is what's expected.
Note that this situation should never occur, since not only do we
require the certificate to be newline terminated, but the signature will
only be read from the beginning of a line. Nevertheless, it seems
prudent to correct it.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git describe --debug localizes all debug messages but not the terms
head, lightweight, annotated that it outputs for the candidates.
Localize them, too.
Signed-off-by: Michael J Gruber <git@grubix.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git describe --dirty" dies when it cannot be determined if the
state in the working tree matches that of HEAD (e.g. broken
repository or broken submodule). The command learned a new option
"git describe --broken" to give "$name-broken" (where $name is the
description of HEAD) in such a case.
* sb/describe-broken:
builtin/describe: introduce --broken flag
Recently we started passing the "--push-options" through the
external remote helper interface; now the "smart HTTP" remote
helper understands what to do with the passed information.
* sb/push-options-via-transport:
remote-curl: allow push options
send-pack: send push options correctly in stateless-rpc case
Code clean-up with minor bugfixes.
* jk/prefix-filename:
bundle: use prefix_filename with bundle path
prefix_filename: simplify windows #ifdef
prefix_filename: return newly allocated string
prefix_filename: drop length parameter
prefix_filename: move docstring to header file
hash-object: fix buffer reuse with --path in a subdirectory
Convert the caller of sha1_array_append to struct object_id.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since we will likely be introducing a new hash function at some point,
and that hash function might be longer than 20 bytes, use the constant
GIT_MAX_RAWSZ, which is designed to be suitable for allocations, instead
of GIT_SHA1_RAWSZ. This will ease the transition down the line by
distinguishing between places where we need to allocate memory suitable
for the largest hash from those where we need to handle the current
hash.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since we will likely be introducing a new hash function at some point,
and that hash function might be longer than 40 hex characters, use the
constant GIT_MAX_HEXSZ, which is designed to be suitable for
allocations, instead of GIT_SHA1_HEXSZ. This will ease the transition
down the line by distinguishing between places where we need to allocate
memory suitable for the largest hash from those where we need to handle
the current hash.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The code to parse the command line "git grep <patterns>... <rev>
[[--] <pathspec>...]" has been cleaned up, and a handful of bugs
have been fixed (e.g. we used to check "--" if it is a rev).
* jk/grep-no-index-fix:
grep: treat revs the same for --untracked as for --no-index
grep: do not diagnose misspelt revs with --no-index
grep: avoid resolving revision names in --no-index case
grep: fix "--" rev/pathspec disambiguation
grep: re-order rev-parsing loop
grep: do not unnecessarily query repo for "--"
grep: move thread initialization a little lower
"git push" had a handful of codepaths that could lead to a deadlock
when unexpected error happened, which has been fixed.
* jk/push-deadlock-regression-fix:
send-pack: report signal death of pack-objects
send-pack: read "unpack" status even on pack-objects failure
send-pack: improve unpack-status error messages
send-pack: use skip_prefix for parsing unpack status
send-pack: extract parsing of "unpack" response
receive-pack: fix deadlock when we cannot create tmpdir
Several callers use fixed buffers for storing the pack
object header, and they've picked 10 as a magic number. This
is reasonable, since it handles objects up to 2^67. But
let's give them a constant so it's clear that the number
isn't pulled out of thin air.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The encode_in_pack_object_header() writes a variable-length
header to an output buffer, but it doesn't actually know
long the buffer is. At first glance, this looks like it
might be possible to overflow.
In practice, this is probably impossible. The smallest
buffer we use is 10 bytes, which would hold the header for
an object up to 2^67 bytes. Obviously we're not likely to
see such an object, but we might worry that an object could
lie about its size (causing us to overflow before we realize
it does not actually have that many bytes). But the argument
is passed as a uintmax_t. Even on systems that have __int128
available, uintmax_t is typically restricted to 64-bit by
the ABI.
So it's unlikely that a system exists where this could be
exploited. Still, it's easy enough to use a normal out/len
pair and make sure we don't write too far. That protects the
hypothetical 128-bit system, makes it harder for callers to
accidentally specify a too-small buffer, and makes the
resulting code easier to audit.
Note that the one caller in fast-import tried to catch such
a case, but did so _after_ the call (at which point we'd
have already overflowed!). This check can now go away.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the tag, branch & for-each-ref commands to have a --no-contains
option in addition to their longstanding --contains options.
This allows for finding the last-good rollout tag given a known-bad
<commit>. Given a hypothetically bad commit cf5c7253e0, the git
version to revert to can be found with this hacky two-liner:
(git tag -l 'v[0-9]*'; git tag -l --contains cf5c7253e0 'v[0-9]*') |
sort | uniq -c | grep -E '^ *1 ' | awk '{print $2}' | tail -n 10
With this new --no-contains option the same can be achieved with:
git tag -l --no-contains cf5c7253e0 'v[0-9]*' | sort | tail -n 10
As the filtering machinery is shared between the tag, branch &
for-each-ref commands, implement this for those commands too. A
practical use for this with "branch" is e.g. finding branches which
were branched off between v2.8.0 and v2.10.0:
git branch --contains v2.8.0 --no-contains v2.10.0
The "describe" command also has a --contains option, but its semantics
are unrelated to what tag/branch/for-each-ref use --contains for. A
--no-contains option for "describe" wouldn't make any sense, other
than being exactly equivalent to not supplying --contains at all,
which would be confusing at best.
Add a --without option to "tag" as an alias for --no-contains, for
consistency with --with and --contains. The --with option is
undocumented, and possibly the only user of it is
Junio (<xmqqefy71iej.fsf@gitster.mtv.corp.google.com>). But it's
trivial to support, so let's do that.
The additions to the the test suite are inverse copies of the
corresponding --contains tests. With this change --no-contains for
tag, branch & for-each-ref is just as well tested as the existing
--contains option.
In addition to those tests, add a test for "tag" which asserts that
--no-contains won't find tree/blob tags, which is slightly
unintuitive, but consistent with how --contains works & is documented.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the --points-at option to default to HEAD for consistency with
its siblings --contains, --merged etc. which default to
HEAD. Previously we'd get:
$ git tag --points-at 2>&1 | head -n 1
error: option `points-at' requires a value
This changes behavior added in commit ae7706b9ac (tag: add --points-at
list option, 2012-02-08).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the "tag" command to implicitly turn on its --list mode when
provided with a list-like option such as --contains, --points-at etc.
This is for consistency with how "branch" works. When "branch" is
given a list-like option, such as --contains, it implicitly provides
--list. Before this change "tag" would error out on those sorts of
invocations. I.e. while both of these worked for "branch":
git branch --contains v2.8.0 <pattern>
git branch --list --contains v2.8.0 <pattern>
Only the latter form worked for "tag":
git tag --contains v2.8.0 '*rc*'
git tag --list --contains v2.8.0 '*rc*'
Now "tag", like "branch", will implicitly supply --list when a
list-like option is provided, and no other conflicting non-list
options (such as -d) are present on the command-line.
Spelunking through the history via:
git log --reverse -p -G'only allowed with' -- '*builtin*tag*c'
Reveals that there was no good reason for not allowing this in the
first place. The --contains option added in 32c35cfb1e ("git-tag: Add
--contains option", 2009-01-26) made this an error. All the other
subsequent list-like options that were added copied its pattern of
making this usage an error.
The only tests that break as a result of this change are tests that
were explicitly checking that this "branch-like" usage wasn't
permitted. Change those failing tests to check that this invocation
mode is permitted, add extra tests for the list-like options we
weren't testing, and tests to ensure that e.g. we don't toggle the
list mode in the presence of other conflicting non-list options.
With this change errors messages such as "--contains option is only
allowed with -l" don't make sense anymore, since options like
--contain turn on -l. Instead we error out when list-like options such
as --contain are used in conjunction with conflicting options such as
-d or -v.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change mentions of <object> to <commit> in the help output of
for-each-ref as appropriate.
Both --[no-]merged and --contains only take commits, but --points-at
can take any object, such as a tag pointing to a tree or blob.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teach remote-curl to understand push options and to be able to convey
them across HTTP.
Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-describe tells you the version number you're at, or errors out, e.g.
when you run it outside of a repository, which may happen when downloading
a tar ball instead of using git to obtain the source code.
To keep this property of only erroring out, when not in a repository,
severe (submodule) errors must be downgraded to reporting them gently
instead of having git-describe error out completely.
To achieve that a flag '--broken' is introduced, which is in the same
vein as '--dirty' but uses an actual child process to check for dirtiness.
When that child dies unexpectedly, we'll append '-broken' instead of
'-dirty'.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Code clean-up.
* jk/pack-name-cleanups:
index-pack: make pointer-alias fallbacks safer
replace snprintf with odb_pack_name()
odb_pack_keep(): stop generating keepfile name
sha1_file.c: make pack-name helper globally accessible
move odb_* declarations out of git-compat-util.h
"git show-branch" expected there were only very short branch names
in the repository and used a fixed-length buffer to hold them
without checking for overflow.
* jk/show-branch-lift-name-len-limit:
show-branch: use skip_prefix to drop magic numbers
show-branch: store resolved head in heap buffer
show-branch: drop head_len variable
"git remote rm X", when a branch has remote X configured as the
value of its branch.*.remote, tried to remove branch.*.remote and
branch.*.merge and failed if either is unset.
* rl/remote-allow-missing-branch-name-merge:
remote: ignore failure to remove missing branch.<name>.merge
A "gc.log" file left by a backgrounded "gc --auto" disables further
automatic gc; it has been taught to run at least once a day (by
default) by ignoring a stale "gc.log" file that is too old.
* dt/gc-ignore-old-gc-logs:
gc: ignore old gc.log files
We may take the path to a bundle file as an argument, and
need to adjust the filename based on the prefix we
discovered while setting up the git directory. We do so
manually into a fixed-size buffer, but using
prefix_filename() is the normal way.
Besides being more concise, there are two subtle
improvements:
1. The original inserted a "/" between the two paths, even
though the "prefix" argument always has the "/"
appended. That means that:
cd subdir && git bundle verify ../foo.bundle
was looking at (and reporting) subdir//../foo.bundle.
Harmless, but ugly. Using prefix_filename() gets this
right.
2. The original checked for an absolute path by looking
for a leading '/'. It should have been using
is_absolute_path(), which also covers more cases on
Windows (backslashes and dos drive prefixes).
But it's easier still to just pass the name to
prefix_filename(), which handles this case
automatically.
Note that we'll just leak the resulting buffer in the name
of simplicity, since it needs to last through the duration
of the program anyway.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The prefix_filename() function returns a pointer to static
storage, which makes it easy to use dangerously. We already
fixed one buggy caller in hash-object recently, and the
calls in apply.c are suspicious (I didn't dig in enough to
confirm that there is a bug, but we call the function once
in apply_all_patches() and then again indirectly from
parse_chunk()).
Let's make it harder to get wrong by allocating the return
value. For simplicity, we'll do this even when the prefix is
empty (and we could just return the original file pointer).
That will cause us to allocate sometimes when we wouldn't
otherwise need to, but this function isn't called in
performance critical code-paths (and it already _might_
allocate on any given call, so a caller that cares about
performance is questionable anyway).
The downside is that the callers need to remember to free()
the result to avoid leaking. Most of them already used
xstrdup() on the result, so we know they are OK. The
remainder have been converted to use free() as appropriate.
I considered retaining a prefix_filename_unsafe() for cases
where we know the static lifetime is OK (and handling the
cleanup is awkward). This is only a handful of cases,
though, and it's not worth the mental energy in worrying
about whether the "unsafe" variant is OK to use in any
situation.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This function takes the prefix as a ptr/len pair, but in
every caller the length is exactly strlen(ptr). Let's
simplify the interface and just take the string. This saves
callers specifying it (and in some cases handling a NULL
prefix).
In a handful of cases we had the length already without
calling strlen, so this is technically slower. But it's not
likely to matter (after all, if the prefix is non-empty
we'll allocate and copy it into a buffer anyway).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The hash-object command uses prefix_filename() without
duplicating its return value. Since that function returns a
static buffer, the value is overwritten by subsequent calls.
This can cause incorrect results when we use --path along
with hashing a file by its relative path, both of which need
to call prefix_filename(). We overwrite the filename
computed for --path, effectively ignoring it.
We can fix this by calling xstrdup on the return value. Note
that we don't bother freeing the "vpath" instance, as it
remains valid until the program exit.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git difftool --dir-diff" used to die a controlled death giving a
"fatal" message when encountering a locally modified symbolic link,
but it started segfaulting since v2.12. This has been fixed.
* js/difftool-builtin:
difftool: handle modified symlinks in dir-diff mode
t7800: cleanup cruft left behind by tests
t7800: remove whitespace before redirect
The string after_subject is added to a strbuf by pp_title_line() if
it's not NULL. Adding an empty string has the same effect as not
adding anything, but the latter is easier, so don't bother changing
the context member from NULL to "".
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of counting the arguments to see if there are any and then
building the full command use a single loop and add the hook command
just before the first argument. This reduces duplication and overall
code size.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
