The second batch of the transactional ref update series.
* rs/ref-transaction-1: (22 commits)
update-ref --stdin: pass transaction around explicitly
update-ref --stdin: narrow scope of err strbuf
refs.c: make delete_ref use a transaction
refs.c: make prune_ref use a transaction to delete the ref
refs.c: remove lock_ref_sha1
refs.c: remove the update_ref_write function
refs.c: remove the update_ref_lock function
refs.c: make lock_ref_sha1 static
walker.c: use ref transaction for ref updates
fast-import.c: use a ref transaction when dumping tags
receive-pack.c: use a reference transaction for updating the refs
refs.c: change update_ref to use a transaction
branch.c: use ref transaction for all ref updates
fast-import.c: change update_branch to use ref transactions
sequencer.c: use ref transactions for all ref updates
commit.c: use ref transactions for updates
replace.c: use the ref transaction functions for updates
tag.c: use ref transactions when doing updates
refs.c: add transaction.status and track OPEN/CLOSED
refs.c: make ref_transaction_begin take an err argument
...
Change create_branch to use a ref transaction when creating the new branch.
This also fixes a race condition in the old code where two concurrent
create_branch could race since the lock_any_ref_for_update/write_ref_sha1
did not protect against the ref already existing. I.e. one thread could end up
overwriting a branch even if the forcing flag is false.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use `git_config_get_string()` instead of `git_config()` to take advantage of
the config-set API which provides a cleaner control flow. While we are at
it, return -1 if we find no value for the queried variable. Original code
returned 0 for all cases, which was checked by `add_branch_desc()` in
fmt-merge-msg.c resulting in addition of a spurious newline to the `out`
strbuf. Now, the newline addition is skipped as -1 is returned to the caller
if no value is found.
Signed-off-by: Tanay Abhra <tanayabh@gmail.com>
Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently `git_config()` returns an integer signifying an error code.
During rewrites of the function most of the code was shifted to
`git_config_with_options()`. `git_config_with_options()` normally
returns positive values if its `config_source` parameter is set as NULL,
as most errors are fatal, and non-fatal potential errors are guarded
by "if" statements that are entered only when no error is possible.
Still a negative value can be returned in case of race condition between
`access_or_die()` & `git_config_from_file()`. Also, all callers of
`git_config()` ignore the return value except for one case in branch.c.
Change `git_config()` return value to void and make it die if it receives
a negative value from `git_config_with_options()`.
Original-patch-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Tanay Abhra <tanayabh@gmail.com>
Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The skip_prefix() function returns a pointer to the content
past the prefix, or NULL if the prefix was not found. While
this is nice and simple, in practice it makes it hard to use
for two reasons:
1. When you want to conditionally skip or keep the string
as-is, you have to introduce a temporary variable.
For example:
tmp = skip_prefix(buf, "foo");
if (tmp)
buf = tmp;
2. It is verbose to check the outcome in a conditional, as
you need extra parentheses to silence compiler
warnings. For example:
if ((cp = skip_prefix(buf, "foo"))
/* do something with cp */
Both of these make it harder to use for long if-chains, and
we tend to use starts_with() instead. However, the first line
of "do something" is often to then skip forward in buf past
the prefix, either using a magic constant or with an extra
strlen(3) (which is generally computed at compile time, but
means we are repeating ourselves).
This patch refactors skip_prefix() to return a simple boolean,
and to provide the pointer value as an out-parameter. If the
prefix is not found, the out-parameter is untouched. This
lets you write:
if (skip_prefix(arg, "foo ", &arg))
do_foo(arg);
else if (skip_prefix(arg, "bar ", &arg))
do_bar(arg);
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The install_branch_config() function reimplemented the skip_prefix()
function inline.
Reported-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Brian Gesiak <modocache@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since commit 6f084a56 the length of a newly tracked branch name was limited
to 1019 = 1024 - 7 - 7 - 1 characters, a bound derived by having to store
this name in a char[1024] called key with two strings of length at most 7
and a '\0' character.
This was no longer necessary as of commit a9f2c136, which uses a strbuf
(documented in Documentation/technical/api-strbuf.txt) to store this value.
Remove this unneeded check to allow branch names longer than 1019
characters.
Signed-off-by: Jacopo Notarstefano <jacopo.notarstefano@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Leaving only the function definitions and declarations so that any
new topic in flight can still make use of the old functions, replace
existing uses of the prefixcmp() and suffixcmp() with new API
functions.
The change can be recreated by mechanically applying this:
$ git grep -l -e prefixcmp -e suffixcmp -- \*.c |
grep -v strbuf\\.c |
xargs perl -pi -e '
s|!prefixcmp\(|starts_with\(|g;
s|prefixcmp\(|!starts_with\(|g;
s|!suffixcmp\(|ends_with\(|g;
s|suffixcmp\(|!ends_with\(|g;
'
on the result of preparatory changes in this series.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git branch --track" had a minor regression in v1.8.3.2 and later
that made it impossible to base your local work on anything but a
local branch of the upstream repository you are tracking from.
* jh/checkout-auto-tracking:
t3200: fix failure on case-insensitive filesystems
branch.c: Relax unnecessary requirement on upstream's remote ref name
t3200: Add test demonstrating minor regression in 41c21f2
Refer to branch.<name>.remote/merge when documenting --track
t3200: Minor fix when preparing for tracking failure
t2024: Fix &&-chaining and a couple of typos
Give "update-refs" a "--stdin" option to read multiple update
requests and perform them in an all-or-none fashion.
* bk/refs-multi-update:
update-ref: add test cases covering --stdin signature
update-ref: support multiple simultaneous updates
refs: add update_refs for multiple simultaneous updates
refs: add function to repack without multiple refs
refs: factor delete_ref loose ref step into a helper
refs: factor update_ref steps into helpers
refs: report ref type from lock_any_ref_for_update
reset: rename update_refs to reset_refs
Fix a minor regression in v1.8.3.2 and later that made it
impossible to base your local work on anything but a local branch
of the upstream repository you are tracking from.
* jh/checkout-auto-tracking:
t3200: fix failure on case-insensitive filesystems
branch.c: Relax unnecessary requirement on upstream's remote ref name
t3200: Add test demonstrating minor regression in 41c21f2
Refer to branch.<name>.remote/merge when documenting --track
t3200: Minor fix when preparing for tracking failure
t2024: Fix &&-chaining and a couple of typos
When creating an upstream relationship, we use the configured remotes and
their refspecs to determine the upstream configuration settings
branch.<name>.remote and branch.<name>.merge. However, if the matching
refspec does not have refs/heads/<something> on the remote side, we end
up rejecting the match, and failing the upstream configuration.
It could be argued that when we set up an branch's upstream, we want that
upstream to also be a proper branch in the remote repo. Although this is
typically the common case, there are cases (as demonstrated by the previous
patch in this series) where this requirement prevents a useful upstream
relationship from being formed. Furthermore:
- We have fundamentally no say in how the remote repo have organized its
branches. The remote repo may put branches (or branch-like constructs
that are insteresting for downstreams to track) outside refs/heads/*.
- The user may intentionally want to track a non-branch from a remote
repo, by using a branch and configured upstream in the local repo.
Relaxing the checking to only require a matching remote/refspec allows the
testcase introduced in the previous patch to succeed, and has no negative
effect on the rest of the test suite.
This patch fixes a behavior (arguably a regression) first introduced in
41c21f2 (branch.c: Validate tracking branches with refspecs instead of
refs/remotes/*) on 2013-04-21 (released in >= v1.8.3.2).
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Expose lock_ref_sha1_basic's type_p argument to callers of
lock_any_ref_for_update. Update all call sites to ignore it by passing
NULL for now.
Signed-off-by: Brad King <brad.king@kitware.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update "git checkout foo" that DWIMs the intended "upstream" and
turns it into "git checkout -t -b foo remotes/origin/foo" to
correctly take existing remote definitions into account.
The remote "origin" may be what uniquely map its own branch to
remotes/some/where/foo but that some/where may not be "origin".
* jh/checkout-auto-tracking:
glossary: Update and rephrase the definition of a remote-tracking branch
branch.c: Validate tracking branches with refspecs instead of refs/remotes/*
t9114.2: Don't use --track option against "svn-remote"-tracking branches
t7201.24: Add refspec to keep --track working
t3200.39: tracking setup should fail if there is no matching refspec.
checkout: Use remote refspecs when DWIMming tracking branches
t2024: Show failure to use refspec when DWIMming remote branch names
t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'
The current code for validating tracking branches (e.g. the argument to
the -t/--track option) hardcodes refs/heads/* and refs/remotes/* as the
potential locations for tracking branches. This works with the refspecs
created by "git clone" or "git remote add", but is suboptimal in other
cases:
- If "refs/remotes/foo/bar" exists without any association to a remote
(i.e. there is no remote named "foo", or no remote with a refspec
that matches "refs/remotes/foo/bar"), then it is impossible to set up
a valid upstream config that tracks it. Currently, the code defaults
to using "refs/remotes/foo/bar" from repo "." as the upstream, which
works, but is probably not what the user had in mind when running
"git branch baz --track foo/bar".
- If the user has tweaked the fetch refspec for a remote to put its
remote-tracking branches outside of refs/remotes/*, e.g. by running
git config remote.foo.fetch "+refs/heads/*:refs/foo_stuff/*"
then the current code will refuse to use its remote-tracking branches
as --track arguments, since they do not match refs/remotes/*.
This patch removes the "refs/remotes/*" requirement for upstream branches,
and replaces it with explicit checking of the refspecs for each remote to
determine whether a given --track argument is a valid remote-tracking
branch. This solves both of the above problems, since the matching refspec
guarantees that there is a both a remote name and a remote branch name
that can be used for the upstream config.
However, this means that refs located within refs/remotes/* without a
corresponding remote/refspec will no longer be usable as upstreams.
The few existing tests which depended on this behavioral quirk has
already been fixed in the preceding patches.
This patch fixes the last remaining test failure in t2024-checkout-dwim.
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If the user requests to --set-upstream-to a branch that does
not exist, then either:
1. It was a typo.
2. They thought the branch should exist.
In case (1), there is not much we can do beyond showing the
name we tried to use. For case (2), though, we can help to
guide them through common workflows.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If we refuse a branch operation because the tracking
start_name the user gave us is bogus, we just print
something like:
fatal: Cannot setup tracking information; start point is not a branch
If we mention the actual name we tried to use, that may help
the user figure out why it didn't work (e.g., if they gave
us the arguments in the wrong order).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If we are trying to set the upstream config for a branch,
the create_branch function will check both that the name
resolves as a ref, and that it is either a local or
remote-tracking branch.
However, before we do so we run get_sha1 on it to find out
whether it resolves at all (since the create_branch function
is also used to create actual branches, it wants to know
where to start the new branch). This means that if you feed
a ref that does not exist to "branch --set-upstream-to",
rather than getting a helpful message about tracking, you
only get "not a valid object name".
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This message is duplicated, and is quite long. Let's factor
it out, which avoids the repetition and the long lines. It
will also make future patches easier as we tweak the
message.
While we're at it, let's also mark it for translation.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Like the "switched to..." message (which is already
suppressed by "-q"), this message is purely informational.
Let's silence it if the user asked us to be quiet.
This patch is slightly more than a one-liner, because we
have to teach create_branch to propagate the flag all the
way down to install_branch_config.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* nd/resolve-ref:
Rename resolve_ref() to resolve_ref_unsafe()
Convert resolve_ref+xstrdup to new resolve_refdup function
revert: convert resolve_ref() to read_ref_full()
* jn/maint-sequencer-fixes:
revert: stop creating and removing sequencer-old directory
Revert "reset: Make reset remove the sequencer state"
revert: do not remove state until sequence is finished
revert: allow single-pick in the middle of cherry-pick sequence
revert: pass around rev-list args in already-parsed form
revert: allow cherry-pick --continue to commit before resuming
revert: give --continue handling its own function
resolve_ref() may return a pointer to a shared buffer and can be
overwritten by the next resolve_ref() calls. Callers need to
pay attention, not to keep the pointer when the next call happens.
Rename with "_unsafe" suffix to warn developers (or reviewers) before
introducing new call sites.
This patch is generated using the following command
git grep -l 'resolve_ref(' -- '*.[ch]'|xargs sed -i 's/resolve_ref(/resolve_ref_unsafe(/g'
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This reverts commit 95eb88d8ee, which
was a UI experiment that did not reflect how "git reset" actually gets
used. The reversion also fixes a test, indicated in the patch.
Encouraged-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* jc/request-pull-show-head-4:
request-pull: use the annotated tag contents
fmt-merge-msg.c: Fix an "dubious one-bit signed bitfield" sparse error
environment.c: Fix an sparse "symbol not declared" warning
builtin/log.c: Fix an "Using plain integer as NULL pointer" warning
fmt-merge-msg: use branch.$name.description
request-pull: use the branch description
request-pull: state what commit to expect
request-pull: modernize style
branch: teach --edit-description option
format-patch: use branch description in cover letter
branch: add read_branch_desc() helper function
Conflicts:
builtin/branch.c
When on master, "git checkout -B master <commit>" is a more natural way to
say "git reset --keep <commit>", which was originally invented for the
exact purpose of moving to the named commit while keeping the local changes
around.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When conflicts are encountered while reverting a commit, it can be
handy to have the name of that commit easily available. For example,
to produce a copy of the patch to refer to while resolving conflicts:
$ git revert 2eceb2a8
error: could not revert 2eceb2a8... awesome, buggy feature
$ git show -R REVERT_HEAD >the-patch
$ edit $(git diff --name-only)
Set a REVERT_HEAD pseudoref when "git revert" does not make a commit,
for cases like this. This also makes it possible for scripts to
distinguish between a revert that encountered conflicts and other
sources of an unmerged index.
After successfully committing, resetting with "git reset", or moving
to another commit with "git checkout" or "git reset", the pseudoref is
no longer useful, so remove it.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This will be used by various callers that make use of the branch
description throughout the system, so that if we need to update
the implementation the callers do not have to be modified.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* rr/revert-cherry-pick-continue:
builtin/revert.c: make commit_list_append() static
revert: Propagate errors upwards from do_pick_commit
revert: Introduce --continue to continue the operation
revert: Don't implicitly stomp pending sequencer operation
revert: Remove sequencer state when no commits are pending
reset: Make reset remove the sequencer state
revert: Introduce --reset to remove sequencer state
revert: Make pick_commits functionally act on a commit list
revert: Save command-line options for continuing operation
revert: Save data for continuing after conflict resolution
revert: Don't create invalid replay_opts in parse_args
revert: Separate cmdline parsing from functional code
revert: Introduce struct to keep command-line options
revert: Eliminate global "commit" variable
revert: Rename no_replay to record_origin
revert: Don't check lone argument in get_encoding
revert: Simplify and inline add_message_to_msg
config: Introduce functions to write non-standard file
advice: Introduce error_resolve_conflict
The "git branch" command, while not in listing mode, calls create_branch()
even when the target branch already exists, and it does so even when it is
not interested in updating the value of the branch (i.e. the name of the
commit object that sits at the tip of the existing branch). This happens
when the command is run with "--set-upstream" option.
The earlier safety-measure to prevent "git branch -f $branch $commit" from
updating the currently checked out branch did not take it into account,
and we no longer can update the tracking information of the current branch.
Minimally fix this regression by telling the validation code if it is
called to really update the value of a potentially existing branch, or if
the caller merely is interested in updating auxiliary aspects of a branch.
Reported-and-Tested-by: Jay Soffian
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the error message when doing: "git branch @{-1}",
"git checkout -b @{-1}", or "git branch -m foo @{-1}"
* was: A branch named '@{-1}' already exists.
* now: A branch named 'bar' already exists.
Signed-off-by: Conrad Irwin <conrad.irwin@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git branch -M <foo> <current-branch>" allows updating the current branch
which HEAD points, without the necessary house-keeping that git reset
normally does to make this operation sensible. It also leaves the reflog
in a confusing state (you would be warned when trying to read it).
"git checkout -B <current branch> <foo>" is also partly vulnerable to this
bug; due to inconsistent pre-flight checks it would perform half of its
task and then abort just before rewriting the branch. Again this
manifested itself as the index file getting out-of-sync with HEAD.
"git branch -f" already guarded against this problem, and aborts with
a fatal error.
Update "git branch -M", "git checkout -B" and "git branch -f" to share the
same check before allowing a branch to be created. These prevent you from
updating the current branch.
We considered suggesting the use of "git reset" in the failure message
but concluded that it was not possible to discern what the user was
actually trying to do.
Signed-off-by: Conrad Irwin <conrad.irwin@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When setting up tracking info, branch.c uses the given branch specifier
("name"). Use the parsed name ("ref.buf") instead so that
git branch --set-upstream @{-1} foo
sets up tracking info for the previous branch rather than for a branch
named "@{-1}".
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Years of muscle memory have trained users to use "git reset --hard" to
remove the branch state after any sort operation. Make it also remove
the sequencer state to facilitate this established workflow:
$ git cherry-pick foo..bar
... conflict encountered ...
$ git reset --hard # Oops, I didn't mean that
$ git cherry-pick quux..bar
... cherry-pick succeeded ...
Guard against accidental removal of the sequencer state by providing
one level of "undo". In the first "reset" invocation,
".git/sequencer" is moved to ".git/sequencer-old"; it is completely
removed only in the second invocation.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a cherry-pick conflicts git advises:
$ git commit -c <original commit id>
to preserve the original commit message and authorship. Instead, let's
record the original commit id in CHERRY_PICK_HEAD and advise:
$ git commit -c CHERRY_PICK_HEAD
A later patch teaches git to handle the '-c CHERRY_PICK_HEAD' part.
Note that we record CHERRY_PICK_HEAD even in the case where there
are no conflicts so that we may use it to communicate authorship to
commit; this will then allow us to remove set_author_ident_env from
revert.c. However, we do not record CHERRY_PICK_HEAD when --no-commit
is used, as presumably the user intends to further edit the commit
and possibly even cherry-pick additional commits on top.
Tests and documentation contributed by Jonathan Nieder.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When creating a new branch using the --track option, we must make sure that
we don't try to set an upstream that does not make sense to follow (using
'git pull') or update (using 'git push'). The current code checks against
using HEAD as upstream (since tracking a symref doesn't make sense). However,
tracking a tag doesn't make sense either. Indeed, tracking _any_ ref that is
not a (local or remote) branch doesn't make sense, and should be disallowed.
This patch achieves this by checking that the ref we're trying to --track
resides within refs/heads/* or refs/remotes/*. This new check replaces the
previous check against HEAD.
A couple of testcases are also added, verifying that we cannot create
branches with tags as upstreams.
Finally, some selftests relying on using a non-branch as an upstream have
been reworked or removed:
- t6040: Reverse the meaning of two tests that depend on the ability to
use (lightweight and annotated) tags as upstreams. These two tests were
originally added in commits 1be570f and 57ffc5f, and this patch reverts the
intention of those two commits.
- t7201: Remove part of a test (introduced in 9188ed8) relying on a
non-branch as upstream.
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ensure that strcmp() isn't called when head is null.
Previously we were getting segfaults when checkout -B was done from a
detached HEAD.
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previous patch allows commands like "git branch --set-upstream foo foo",
which doesn't make much sense. Warn the user and don't change the
configuration in this case. Don't die to let the caller finish its job in
such case.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
