Instead of open-coding the function pop_commit() just call it. This
makes the intent clearer and reduces code size.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
for_each_ref() callback functions were taught to name the objects
not with "unsigned char sha1[20]" but with "struct object_id".
* bc/object-id: (56 commits)
struct ref_lock: convert old_sha1 member to object_id
warn_if_dangling_symref(): convert local variable "junk" to object_id
each_ref_fn_adapter(): remove adapter
rev_list_insert_ref(): remove unneeded arguments
rev_list_insert_ref_oid(): new function, taking an object_oid
mark_complete(): remove unneeded arguments
mark_complete_oid(): new function, taking an object_oid
clear_marks(): rewrite to take an object_id argument
mark_complete(): rewrite to take an object_id argument
send_ref(): convert local variable "peeled" to object_id
upload-pack: rewrite functions to take object_id arguments
find_symref(): convert local variable "unused" to object_id
find_symref(): rewrite to take an object_id argument
write_one_ref(): rewrite to take an object_id argument
write_refs_to_temp_dir(): convert local variable sha1 to object_id
submodule: rewrite to take an object_id argument
shallow: rewrite functions to take object_id arguments
handle_one_ref(): rewrite to take an object_id argument
add_info_ref(): rewrite to take an object_id argument
handle_one_reflog(): rewrite to take an object_id argument
...
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change typedef each_ref_fn to take a "const struct object_id *oid"
parameter instead of "const unsigned char *sha1".
To aid this transition, implement an adapter that can be used to wrap
old-style functions matching the old typedef, which is now called
"each_ref_sha1_fn"), and make such functions callable via the new
interface. This requires the old function and its cb_data to be
wrapped in a "struct each_ref_fn_sha1_adapter", and that object to be
used as the cb_data for an adapter function, each_ref_fn_adapter().
This is an enormous diff, but most of it consists of simple,
mechanical changes to the sites that call any of the "for_each_ref"
family of functions. Subsequent to this change, the call sites can be
rewritten one by one to use the new interface.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a triangular workflow, the place you pull from and the
place you push to may be different. As we have
branch_get_upstream for the former, this patch adds
branch_get_push for the latter (and as the former implements
@{upstream}, so will this implement @{push} in a future
patch).
Note that the memory-handling for the return value bears
some explanation. Some code paths require allocating a new
string, and some let us return an existing string. We should
provide a consistent interface to the caller, so it knows
whether to free the result or not.
We could do so by xstrdup-ing any existing strings, and
having the caller always free. But that makes us
inconsistent with branch_get_upstream, so we would prefer to
simply take ownership of the resulting string. We do so by
storing it inside the "struct branch", just as we do with
the upstream refname (in that case we compute it when the
branch is created, but there's no reason not to just fill
it in lazily in this case).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After calling stat_tracking_info, callers often want to
print the name of the upstream branch (in addition to the
tracking count). To do this, they have to access
branch->merge->dst[0] themselves. This is not wrong, as the
return value from stat_tracking_info tells us whether we
have an upstream branch or not. But it is a bit leaky, as we
make an assumption about how it calculated the upstream
name.
Instead, let's add an out-parameter that lets the caller
know the upstream name we found.
As a bonus, we can get rid of the unusual tri-state return
from the function. We no longer need to use it to
differentiate between "no tracking config" and "tracking ref
does not exist" (since you can check the upstream_name for
that), so we can just use the usual 0/-1 convention for
success/error.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The error-diagnosis logic in branch_get_upstream was copied
straight from sha1_name.c in the previous commit. However,
because we check all error cases and upfront and then later
diagnose them, the logic is a bit tangled. In particular:
- if branch->merge[0] is NULL, we may end up dereferencing
it for an error message (in practice, it should never be
NULL, so this is probably not a triggerable bug).
- We may enter the code path because branch->merge[0]->dst
is NULL, but we then start our error diagnosis by
checking whether our local branch exists. But that is
only relevant to diagnosing missing merge config, not a
missing tracking ref; our diagnosis may hide the real
problem.
Instead, let's just use a sequence of "if" blocks to check
for each error type, diagnose it, and return NULL.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the previous commit introduced the branch_get_upstream
helper, there was one call-site that could not be converted:
the one in sha1_name.c, which gives detailed error messages
for each possible failure.
Let's teach the helper to optionally report these specific
errors. This lets us convert another callsite, and means we
can use the helper in other locations that want to give the
same error messages.
The logic and error messages come straight from sha1_name.c,
with the exception that we start each error with a lowercase
letter, as is our usual style (note that a few tests need
updated as a result).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
All of the information needed to find the @{upstream} of a
branch is included in the branch struct, but callers have to
navigate a series of possible-NULL values to get there.
Let's wrap that logic up in an easy-to-read helper.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Before the previous commit, we had to make sure that
read_config() was called before entering remote_get_1,
because we needed to pass pushremote_name by value. But now
that we pass a function, we can let remote_get_1 handle
loading the config itself, turning our wrappers into true
one-liners.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When remote.c loads its config, it records the
branch.*.pushremote for the current branch along with the
global remote.pushDefault value, and then binds them into a
single value: the default push for the current branch. We
then pass this value (which may be NULL) to remote_get_1
when looking up a remote for push.
This has a few downsides:
1. It's confusing. The early-binding of the "current
value" led to bugs like the one fixed by 98b406f
(remote: handle pushremote config in any order,
2014-02-24). And the fact that pushremotes fall back to
ordinary remotes is not explicit at all; it happens
because remote_get_1 cannot tell the difference between
"we are not asking for the push remote" and "there is
no push remote configured".
2. It throws away intermediate data. After read_config()
finishes, we have no idea what the value of
remote.pushDefault was, because the string has been
overwritten by the current branch's
branch.*.pushremote.
3. It doesn't record other data. We don't note the
branch.*.pushremote value for anything but the current
branch.
Let's make this more like the fetch-remote config. We'll
record the pushremote for each branch, and then explicitly
compute the correct remote for the current branch at the
time of reading.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We'll want to use this logic as a fallback when looking up
the pushremote, so let's pull it out into its own function.
We don't technically need to make this available outside of
remote.c, but doing so will provide a consistent API with
pushremote_for_branch, which we will add later.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we create each branch struct, we fill in the
"remote_name" field from the config, and then fill in the
actual "remote" field (with a "struct remote") based on that
name. However, it turns out that nobody really cares about
the latter field. The only two sites that access it at all
are:
1. git-merge, which uses it to notice when the branch does
not have a remote defined. But we can easily replace this
with looking at remote_name instead.
2. remote.c itself, when setting up the @{upstream} merge
config. But we don't need to save the "remote" in the
"struct branch" for that; we can just look it up for
the duration of the operation.
So there is no need to have both fields; they are redundant
with each other (the struct remote contains the name, or you
can look up the struct from the name). It would be nice to
simplify this, especially as we are going to add matching
pushremote config in a future patch (and it would be nice to
keep them consistent).
So which one do we keep and which one do we get rid of?
If we had a lot of callers accessing the struct, it would be
more efficient to keep it (since you have to do a lookup to
go from the name to the struct, but not vice versa). But we
don't have a lot of callers; we have exactly one, so
efficiency doesn't matter. We can decide this based on
simplicity and readability.
And the meaning of the struct value is somewhat unclear. Is
it always the remote matching remote_name? If remote_name is
NULL (i.e., no per-branch config), does the struct fall back
to the "origin" remote, or is it also NULL? These questions
will get even more tricky with pushremotes, whose fallback
behavior is more complicated. So let's just store the name,
which pretty clearly represents the branch.*.remote config.
Any lookup or fallback behavior can then be implemented in
helper functions.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we call branch_get() to lookup or create a "struct
branch", we make sure the "merge" field is filled in so that
callers can access it. But the conditions under which we do
so are a little confusing, and can lead to two funny
situations:
1. If there's no branch.*.remote config, we cannot provide
branch->merge (because it is really just an application
of branch.*.merge to our remote's refspecs). But
branch->merge_nr may be non-zero, leading callers to be
believe they can access branch->merge (e.g., in
branch_merge_matches and elsewhere).
It doesn't look like this can cause a segfault in
practice, as most code paths dealing with merge config
will bail early if there is no remote defined. But it's
a bit of a dangerous construct.
We can fix this by setting merge_nr to "0" explicitly
when we realize that we have no merge config. Note that
merge_nr also counts the "merge_name" fields (which we
_do_ have; that's how merge_nr got incremented), so we
will "lose" access to them, in the sense that we forget
how many we had. But no callers actually care; we use
merge_name only while iteratively reading the config,
and then convert it to the final "merge" form the first
time somebody calls branch_get().
2. We set up the "merge" field every time branch_get is
called, even if it has already been done. This leaks
memory.
It's not a big deal in practice, since most code paths
will access only one branch, or perhaps each branch
only one time. But if you want to be pathological, you
can leak arbitrary memory with:
yes @{upstream} | head -1000 | git rev-list --stdin
We can fix this by skipping setup when branch->merge is
already non-NULL.
In addition to those two fixes, this patch pushes the "do we
need to setup merge?" logic down into set_merge, where it is
a bit easier to follow.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we read the remote config from disk, we update a
default_remote_name variable if we see branch.*.remote
config for the current branch. This isn't wrong, or even all
that complicated, but it is a bit simpler (because it
reduces our overall state) to just lazily compute the
default when we need it.
The ulterior motive here is that the push config uses a
similar structure, and _is_ much more complicated as a
result. That will be simplified in a future patch, and it's
more readable if the logic for remotes and push-remotes
matches.
Note that we also used default_remote_name as a signal that
the remote config has been loaded; after this patch, we now
use an explicit flag.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Mark file-local symbols as "static", and drop functions that nobody
uses.
* jc/unused-symbols:
shallow.c: make check_shallow_file_for_update() static
remote.c: make clear_cas_option() static
urlmatch.c: make match_urls() static
revision.c: make save_parents() and free_saved_parents() static
line-log.c: make line_log_data_init() static
pack-bitmap.c: make pack_bitmap_filename() static
prompt.c: remove git_getpass() nobody uses
http.c: make finish_active_slot() and handle_curl_result() static
"git blame HEAD -- missing" failed to correctly say "HEAD" when it
tried to say "No such path 'missing' in HEAD".
* jk/blame-commit-label:
blame.c: fix garbled error message
use xstrdup_or_null to replace ternary conditionals
builtin/commit.c: use xstrdup_or_null instead of envdup
builtin/apply.c: use xstrdup_or_null instead of null_strdup
git-compat-util: add xstrdup_or_null helper
This replaces "x ? xstrdup(x) : NULL" with xstrdup_or_null(x).
The change is fairly mechanical, with the exception of
resolve_refdup, which can eliminate a temporary variable.
There are still a few hits grepping for "?.*xstrdup", but
these are of slightly different forms and cannot be
converted (e.g., "x ? xstrdup(x->foo) : NULL").
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The report from "git checkout" on a branch that builds on another
local branch by setting its branch.*.merge to branch name (not a
full refname) incorrectly said that the upstream is gone.
* jc/checkout-local-track-report:
checkout: report upstream correctly even with loosely defined branch.*.merge
The new name is more consistent with the names of other
string_list-related functions.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
resolve_ref_unsafe takes a boolean argument for reading (a nonexistent ref
resolves successfully for writing but not for reading). Change this to be
a flags field instead, and pass the new constant RESOLVE_REF_READING when
we want this behaviour.
While at it, swap two of the arguments in the function to put output
arguments at the end. As a nice side effect, this ensures that we can
catch callers that were unaware of the new API so they can be audited.
Give the wrapper functions resolve_refdup and read_ref_full the same
treatment for consistency.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When checking out a branch that is set to build on top of another
branch (often, a remote-tracking branch), "git checkout" reports how
your work relates to the other branch, e.g.
Your branch is behind 'origin/master', and can be fast-forwarded.
Back when this feature was introduced, this was only done for
branches that build on remote-tracking branches, but 5e6e2b48 (Make
local branches behave like remote branches when --tracked,
2009-04-01) added support to give the same report for branches that
build on other local branches (i.e. branches whose branch.*.remote
variables are set to '.'). Unlike the support for the branches
building on remote-tracking branches, however, this did not take
into account the fact that branch.*.merge configuration is allowed
to record a shortened branch name.
When branch.*.merge is set to 'master' (not 'refs/heads/master'),
i.e. "my branch builds on the local 'master' branch", this caused
"git checkout" to report:
Your branch is based on 'master', but the upstream is gone.
The upstream is our repository and is definitely not gone, so this
output is nonsense.
The fix is fairly obvious; just like the branch name is DWIMed when
"git pull" merges from the 'master' branch without complaint on such
a branch, the name of the branch the current branch builds upon
needs to be DWIMed the same way.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make the code simpler and shorter by avoiding repetitive use of
string length variables and leaving memory allocation to strbuf
functions.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Even the documentation tells us:
You should check if it returns any error (non-zero return
code) and if it does not, you can start using get_revision()
to do the iteration.
In preparation for this commit, I grepped all occurrences of
prepare_revision_walk and added error messages, when there were none.
Signed-off-by: Stefan Beller <stefanbeller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remotes are stored as an array, so looking one up or adding one without
duplication is an O(n) operation. Reading an entire config file full of
remotes is O(n^2) in the number of remotes. For a repository with tens of
thousands of remotes, the running time can hit multiple minutes.
Hash tables are way faster. So we add a hashmap from remote name to
struct remote and use it for all lookups. The time to add a new remote to
a repo that already has 50,000 remotes drops from ~2 minutes to < 1
second.
We retain the old array of remotes so iterators proceed in config-file
order.
Signed-off-by: Patrick Reynolds <patrick.reynolds@github.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* jk/xstrfmt:
setup_git_env(): introduce git_path_from_env() helper
unique_path: fix unlikely heap overflow
walker_fetch: fix minor memory leak
merge: use argv_array when spawning merge strategy
sequencer: use argv_array_pushf
setup_git_env: use git_pathdup instead of xmalloc + sprintf
use xstrfmt to replace xmalloc + strcpy/strcat
use xstrfmt to replace xmalloc + sprintf
use xstrdup instead of xmalloc + strcpy
use xstrfmt in favor of manual size calculations
strbuf: add xstrfmt helper
* jk/skip-prefix:
http-push: refactor parsing of remote object names
imap-send: use skip_prefix instead of using magic numbers
use skip_prefix to avoid repeated calculations
git: avoid magic number with skip_prefix
fetch-pack: refactor parsing in get_ack
fast-import: refactor parsing of spaces
stat_opt: check extra strlen call
daemon: use skip_prefix to avoid magic numbers
fast-import: use skip_prefix for parsing input
use skip_prefix to avoid repeating strings
use skip_prefix to avoid magic numbers
transport-helper: avoid reading past end-of-string
fast-import: fix read of uninitialized argv memory
apply: use skip_prefix instead of raw addition
refactor skip_prefix to return a boolean
avoid using skip_prefix as a boolean
daemon: mark some strings as const
parse_diff_color_slot: drop ofs parameter
It's a common idiom to match a prefix and then skip past it
with strlen, like:
if (starts_with(foo, "bar"))
foo += strlen("bar");
This avoids magic numbers, but means we have to repeat the
string (and there is no compiler check that we didn't make a
typo in one of the strings).
We can use skip_prefix to handle this case without repeating
ourselves.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In many parts of the code, we do an ugly and error-prone
malloc like:
const char *fmt = "something %s";
buf = xmalloc(strlen(foo) + 10 + 1);
sprintf(buf, fmt, foo);
This makes the code brittle, and if we ever get the
allocation wrong, is a potential heap overflow. Let's
instead favor xstrfmt, which handles the allocation
automatically, and makes the code shorter and more readable.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert three cases of checking for a constant prefix using memcmp() to
starts_with(). This way there is no need for magic string length
constants and we avoid running over the end of the string should it be
shorter than the prefix.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
xcalloc() takes two arguments: the number of elements and their size.
parse_refspec_internal passes the arguments in reverse order, passing the
size of a refspec, followed by the number to allocate.
Rearrange them so they are in the correct order.
Signed-off-by: Brian Gesiak <modocache@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eradicate mistaken use of "nor" (that is, essentially "nor" used
not in "neither A nor B" ;-)) from in-code comments, command output
strings, and documentations.
* jl/nor-or-nand-and:
code and test: fix misuses of "nor"
comments: fix misuses of "nor"
contrib: fix misuses of "nor"
Documentation: fix misuses of "nor"
Protect refs in a hierarchy that can come from more than one remote
hierarcies from incorrect removal by "git fetch --prune".
* cn/fetch-prune-overlapping-destination:
fetch: handle overlaping refspecs on --prune
fetch: add a failing test for prunning with overlapping refspecs
We need to consider that a remote-tracking branch may match more than
one rhs of a fetch refspec. In such a case, it is not enough to stop at
the first match but look at all of the matches in order to determine
whether a head is stale.
To this goal, introduce a variant of query_refspecs which returns all of
the matching refspecs and loop over those answers to check for
staleness.
Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git push" did not pay attention to branch.*.pushremote if it is
defined earlier than remote.pushdefault; the order of these two
variables in the configuration file should not matter, but it did by
mistake.
* jk/remote-pushremote-config-reading:
remote: handle pushremote config in any order
Catch "git push $there no-such-branch" early.
* jk/detect-push-typo-early:
push: detect local refspec errors early
match_explicit_lhs: allow a "verify only" mode
match_explicit: hoist refspec lhs checks into their own function
"git push" did not pay attention to branch.*.pushremote if it is
defined earlier than remote.pushdefault; the order of these two
variables in the configuration file should not matter, but it did by
mistake.
* jk/remote-pushremote-config-reading:
remote: handle pushremote config in any order
When pushing, we do not even look at our push refspecs until
after we have made contact with the remote receive-pack and
gotten its list of refs. This means that we may go to some
work, including asking the user to log in, before realizing
we have simple errors like "git push origin matser".
We cannot catch all refspec problems, since fully evaluating
the refspecs requires knowing what the remote side has. But
we can do a quick sanity check of the local side and catch a
few simple error cases.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The match_explicit_lhs function has all of the logic
necessary to verify the refspecs without actually doing any
work. This patch lets callers pass a NULL "match" pointer to
indicate they want a "verify only" operation.
For the most part, we just need to avoid writing to the NULL
pointer. However, we also have to refactor the
try_explicit_object_name sub-function; it indicates success by
allocating and returning a new ref. Instead, we give it an
"out" parameter for the match and return a numeric status.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In preparation for being able to check the left-hand side of
our push refspecs separately, this pulls the examination of
them out into its own function. There should be no behavior
change.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The remote we push can be defined either by
remote.pushdefault or by branch.*.pushremote for the current
branch. The order in which they appear in the config file
should not matter to precedence (which should be to prefer
the branch-specific config).
The current code parses the config linearly and uses a
single string to store both values, overwriting any
previous value. Thus, config like:
[branch "master"]
pushremote = foo
[remote]
pushdefault = bar
erroneously ends up pushing to "bar" from the master branch.
We can fix this by storing both values and resolving the
correct value after all config is read.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We used to use two separate rules for the normal ref resolution
dwimming and dwimming done to decide which remote ref to grab. The
third parameter to refname_match() selected which rules to use.
When these two rules were harmonized in
2011-11-04 dd621df9cd refs DWIMmery: use the same rule for both "git fetch" and others
, ref_fetch_rules was #defined to avoid potential breakages for
in-flight topics.
It is now safe to remove the backwards-compatibility code, so remove
refname_match()'s third parameter, make ref_rev_parse_rules private to
refs.c, and remove ref_fetch_rules entirely.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make "git push origin master" update the same ref that would be
updated by our 'master' when "git push origin" (no refspecs) is run
while the 'master' branch is checked out, which makes "git push"
more symmetric to "git fetch" and more usable for the triangular
workflow.
* jc/push-refmap:
push: also use "upstream" mapping when pushing a single ref
push: use remote.$name.push as a refmap
builtin/push.c: use strbuf instead of manual allocation
