i18n: git-sh-setup.sh: mark strings for translation
Positional arguments, such as $0, $1, etc, need to be stored on shell
variables for use in translatable strings, according to gettext manual
[1].
Add git-sh-setup.sh to LOCALIZED_SH variable in Makefile to enable
extraction of string marked for translation by xgettext.
Source git-sh-i18n in git-sh-setup.sh for gettext support.
git-sh-setup.sh is a shell library to be sourced by other shell scripts.
In order to avoid other scripts from sourcing git-sh-i18n twice, remove
line that sources it from them. Not sourcing git-sh-i18n in any script
that uses gettext would lead to failure due to, for instance, gettextln
not being found.
! grep expected actual
test_i18ngrep ! expected actual
The latter syntax is supported by test_i18ngrep defined in
t/test-lib.sh.
Although the test already passes whether GETTEXT_POSION is enabled, use
the i18n grep variant for the sake of consistency and also to make
obvious that those strings are subject to i18n.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The message was not being extracted by xgettext, although it was marked
for translation, seemingly because it contained a command substitution.
Moreover, eval_gettext should be used instead of gettext for strings
with substitution.
See step 4. of section 15.5.2.1 Preparing Shell Scripts for
Internationalization from gettext manual [1]:
"Simplify translatable strings so that they don't contain command
substitution ("`...`" or "$(...)") [...]"
The test t9003-help-autocorrect.sh fails when run under GETTEXT_POISON,
because it's expecting to filter out the original output. Accommodate
gettext poison case by also filtering out the default simulated output.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Mark placeholder "<branch>" in git-rebase.sh for translation. The string
containing the named placeholder is passed to shell function
error_on_missing_default_upstream in git-parse-remote.sh which uses it
to display a command hint for the user.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
tests: unpack-trees: update to use test_i18n* functions
Use functions test_i18ncmp and test_i18ngrep to successfully pass tests
running under GETTEXT_POISON.
The output strings compared to in these test were marked for translation
in ed47fdf ("i18n: unpack-trees: mark strings for translation",
2016-04-09) and later improved in 2e3926b ("i18n: unpack-trees: avoid
substituting only a verb in sentences", 2016-05-12).
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
i18n: rebase: fix marked string to use eval_gettext variant
The string message marked for translation should use eval_gettext
variant instead of the gettext one, since we want to dollar-substitute
$head_name in the result.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
tests: use test_i18n* functions to suppress false positives
The test functions test_i18ncmp and test_i18ngrep pretend success if run
under GETTEXT_POISON. By using those functions to test output which is
correctly marked as translatable, enables one to detect if the strings
newly marked for translation are from plumbing output. If they are
indeed from plumbing, the test would fail, and the string should be
unmarked, since it is not seen by users.
Thus, it is productive to not have false positives when running the test
under GETTEXT_POISON. This commit replaces normal test functions by
their i18n aware variants in use-cases know to be correctly marked for
translation, suppressing false positives.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
merge-octopus: use die shell function from git-sh-setup.sh
Source git-sh-setup in order to use die shell function from
git-sh-setup.sh library instead of using the one defined in
git-merge-octopus.sh. Remove the former die function.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
i18n: rebase-interactive: mark comments of squash for translation
Mark comment messages of squash/fixup file ($squash_msg) for
translation.
Helper functions this_nth_commit_message and skip_nth_commit_message
replace the previous method of making the comment messages (such as
"This is the 2nd commit message:") aided by nth_string helper function.
This step was taken as a workaround to enabled translation of entire
sentences. However, doesn't change any text seen in English by the user,
except for string "The first commit's message is:" which was changed to
match the style of other instances.
The test t3404-rebase-interactive.sh resorts to set_fake_editor which
didn't account for GETTEXT_POISON. Fix it by assuming success when we
find dummy gettext poison output where was supposed to find the first
comment line "This is a combination of $count commits.".
For that same message, use plural aware eval_ngettext instead of
eval_gettext, since other languages have more complex plural forms.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
i18n: rebase-interactive: mark here-doc strings for translation
Use pipe to send gettext output to git stripspace instead of the
original method of using shell here-document, because command
substitution '$(...)' would not take place inside the here-documents.
The exception is the case of the last here-document redirecting to cat,
in which commands substitution works and, thus, is preserved in this
commit.
t3404: adapt test to the strings newly marked for translation
Test t3404-rebase-interactive.sh would fail under GETTEXT_POISON unless
using test_i18ngrep.
Add eval_ngettext fallback functions to be called when running, for
instance, under GETTEXT_POISON. Otherwise, tests would fail under
GETTEXT_POISON, or other build that doesn't support the GNU gettext,
because that function could not be found.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
i18n: sequencer: mark entire sentences for translation
Mark entire sentences of error message rather than assembling one using
placeholders (e.g. "Cannot %s during a %s").
That would facilitate translation work because it is easier to translate
a entire sentence than translating pieces. We would have better
translations at the expense of source code verbosity.
Moreover, translators can now 1) translate the terms "revert" and
"cherry-pick" if they please 2) have more leeway to adapt their
translations.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
i18n: advice: internationalize message for conflicts
Mark message for translation telling the user she has conflicts to
resolve. Expose each particular use case, in order to enable translating
entire sentences which would facilitate translating into other
languages.
Change "Pull" to lowercase to match other instances. Update test
t5520-pull.sh, that relied on the old error message, to use the new one.
Although we loose in source code conciseness, we would gain better
translations because translators can 1) translate the entire sentence,
including those terms concerning Git (committing, merging, etc) 2) have
leeway to adapt to their languages.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
i18n: advice: mark string about detached head for translation
Mark string with advice seen by the user when in detached head.
Update test t7201-co.sh to pass under GETTEXT_POISON build. Pretend
success if the number of lines of "git checkout renamer^" output is not
greater than 1 and test are running under GETTEXT_POISON.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The second string inside _() was not being extracted for translation by
xgettext, meaning that, although the string was passed to gettext, there
was no translation available.
Mark each individual string instead of marking the result of ternary if.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We use different types of signature formats in different places.
Set up the infrastructure and overview to describe them systematically
in our technical documentation.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 9f50d32 introduced a fix for FreeBSD /bin/sh misbehaviour
when dot-sourcing a file containing "return" statements outside of
any function, from a function in another shell script. That issue
affects FreeBSD 9.x, and is not present in the /bin/sh in FreeBSD
10.3 and later. Update the comment to clarify this.
The example from 9f50d32's commit message produces the expected output
on FreeBSD 10.3 and -CURRENT (the upcoming 11.0):
% sh script1.sh
only this line should show
%
Signed-off-by: Ed Maste <emaste@freebsd.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
bisect: always call setup_revisions after init_revisions
init_revisions() initializes the rev_info struct to default
values, and setup_revisions() parses any command-line
arguments and finalizes the struct.
In e22278c (bisect: display first bad commit without forking
a new process, 2009-05-28), a show_diff_tree() was added
that calls the former but not the latter. It doesn't have
any arguments to parse, but it still should do the
finalizing step.
This may have caused other minor bugs over the years, but it
became much more prominent after fe37a9c (pretty: allow
tweaking tabwidth in --expand-tabs, 2016-03-29). That leaves
the expected tab width as "-1", rather than the true default
of "8". When we see a commit with tabs to be expanded, we
end up trying to add (size_t)-1 spaces to a strbuf, which
complains about the integer overflow.
The fix is easy: just call setup_revisions() with no
arguments.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pretty.c: support <direction>|(<negative number>) forms
%>|(num), %><|(num) and %<|(num), where num is a positive number, sets a
fixed column from the screen's left border. There is no way for us to
specifiy a column relative to the right border, which is useful when you
want to make use of all terminal space (on big screens). Use negative
num for that. Inspired by Go's array syntax (*).
(*) I know Python has this first (or before Go, at least) but the idea
didn't occur to me until I learned Go.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
this change will make all commit hashes align at 20th column from
the edge of the terminal, not from the edge of the graph.
Signed-off-by: Josef Kufner <josef@kufner.cz> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
upload-pack.c: make send_client_data() return void
The send_client_data() function uses write_or_die() for writing data
which immediately terminates the process on errors. If no such error
occurred, send_client_data() always returned the value that was passed
as third parameter prior to this commit. This value is already known to
the caller in any case, so let's turn send_client_data() into a void
function instead.
Signed-off-by: Lukas Fleischer <lfleischer@lfos.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The send_sideband() function uses write_or_die() for writing data which
immediately terminates the process on errors. If no such error occurred,
send_sideband() always returned the value that was passed as fourth
parameter prior to this commit. This value is already known to the
caller in any case, so let's turn send_sideband() into a void function
instead.
Signed-off-by: Lukas Fleischer <lfleischer@lfos.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We use plumbing to generate the diff, so it doesn't
automatically pick up UI config like compactionHeuristic.
Let's forward it on, since interactive adding is porcelain.
Note that we only need to handle the "true" case. There's no
point in passing --no-compaction-heuristic when the variable
is false, since nothing else could have turned it on.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reported-by: Joseph Pecoraro <pecoraro@apple.com> Signed-off-by: Alfred Perlstein <alfred@freebsd.org> Reviewed-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
repack: extend --keep-unreachable to loose objects
If you use "repack -adk" currently, we will pack all objects
that are already packed into the new pack, and then drop the
old packs. However, loose unreachable objects will be left
as-is. In theory these are meant to expire eventually with
"git prune". But if you are using "repack -k", you probably
want to keep things forever and therefore do not run "git
prune" at all. Meaning those loose objects may build up over
time and end up fooling any object-count heuristics (such as
the one done by "gc --auto", though since git-gc does not
support "repack -k", this really applies to whatever custom
scripts people might have driving "repack -k").
With this patch, we instead stuff any loose unreachable
objects into the pack along with the already-packed
unreachable objects. This may seem wasteful, but it is
really no more so than using "repack -k" in the first place.
We are at a slight disadvantage, in that we have no useful
ordering for the result, or names to hand to the delta code.
However, this is again no worse than what "repack -k" is
already doing for the packed objects. The packing of these
objects doesn't matter much because they should not be
accessed frequently (unless they actually _do_ become
referenced, but then they would get moved to a different
part of the packfile during the next repack).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The usual way to do a full repack (and what is done by
git-gc) is to run "repack -Ad --unpack-unreachable=<when>",
which will loosen any unreachable objects newer than
"<when>", and drop any older ones.
This is a safer alternative to "repack -ad", because
"<when>" becomes a grace period during which we will not
drop any new objects that are about to be referenced.
However, it isn't perfectly safe. It's always possible that
a process is about to reference an old object. Even if that
process were to take care to update the timestamp on the
object, there is no atomicity with a simultaneously running
"repack" process.
So while unlikely, there is a small race wherein we may drop
an object that is in the process of being referenced. If you
do automated repacking on a large number of active
repositories, you may hit it eventually, and the result is a
corrupted repository.
It would be nice to fix that race in the long run, but it's
complicated. In the meantime, there is a much simpler
strategy for automated repository maintenance: do not drop
objects at all. We already have a "--keep-unreachable"
option in pack-objects; we just need to plumb it through
from git-repack.
Note that this _isn't_ plumbed through from git-gc, so at
this point it's strictly a tool for people doing their own
advanced repository maintenance strategy.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This was added back in 7e52f56 (gc: do not explode objects
which will be immediately pruned, 2012-04-07), but not
documented at the time, since it was an internal detail
between git-gc and git-repack. However, as people with
complicated setups may want to effectively reimplement the
steps of git-gc themselves, it is nice for us to document
these interfaces.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
These callers appear to expect that deref_tag() is to peel one layer
of a tag, but the function does not work that way; it has its own
loop to unwrap tags until an object that is not a tag appears.
blame: dwim "blame --reverse OLD" as "blame --reverse OLD.."
Instead of always requiring both ends of a range, we could DWIM
"OLD", which could be a misspelt "OLD..", to be a range that ends at
the current commit.
"git blame --reverse OLD..NEW -- PATH" tells us to start from the
contents in PATH at OLD and observe how each line is changed while
the history develops up to NEW, and report for each line the latest
commit up to which the line survives in the original form.
If you say "git blame --reverse NEW -- PATH" by mistake, we complain
about the missing OLD, but we phrased it as "No commit to dig down
to?" In this case, however, we are digging up from OLD, so say so.
strbuf: describe the return value of strbuf_read_file
Mentored-by: Lars Schneider <larsxschneider@gmail.com> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
fetch: document that pruning happens before fetching
This was changed in 10a6cc8 (fetch --prune: Run prune before
fetching, 2014-01-02), but it seems that nobody in that
discussion realized we were advertising the "after"
explicitly.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
fetch, upload-pack: --deepen=N extends shallow boundary by N commits
In git-fetch, --depth argument is always relative with the latest
remote refs. This makes it a bit difficult to cover this use case,
where the user wants to make the shallow history, say 3 levels
deeper. It would work if remote refs have not moved yet, but nobody
can guarantee that, especially when that use case is performed a
couple months after the last clone or "git fetch --depth". Also,
modifying shallow boundary using --depth does not work well with
clones created by --since or --not.
This patch fixes that. A new argument --deepen=<N> will add <N> more (*)
parent commits to the current history regardless of where remote refs
are.
Have/Want negotiation is still respected. So if remote refs move, the
server will send two chunks: one between "have" and "want" and another
to extend shallow history. In theory, the client could send no "want"s
in order to get the second chunk only. But the protocol does not allow
that. Either you send no want lines, which means ls-remote; or you
have to send at least one want line that carries deep-relative to the
server..
The main work was done by Dongcan Jiang. I fixed it up here and there.
And of course all the bugs belong to me.
(*) We could even support --deepen=<N> where <N> is negative. In that
case we can cut some history from the shallow clone. This operation
(and --depth=<shorter depth>) does not require interaction with remote
side (and more complicated to implement as a result).
Helped-by: Duy Nguyen <pclouds@gmail.com> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Dongcan Jiang <dongcan.jiang@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
upload-pack: support define shallow boundary by excluding revisions
This should allow the user to say "create a shallow clone of this branch
after version <some-tag>".
Short refs are accepted and expanded at the server side with expand_ref()
because we cannot expand (unknown) refs from the client side.
Like deepen-since, deepen-not cannot be used with deepen. But deepen-not
can be mixed with deepen-since. The result is exactly how you do the
command "git rev-list --since=... --not ref".
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is basically dwim_ref() without @{} support. To be used on the
server side where we want to expand abbreviated to full ref names and
nothing else. The first user is "git clone/fetch --shallow-exclude".
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
upload-pack: add deepen-since to cut shallow repos based on time
This should allow the user to say "create a shallow clone containing the
work from last year" (once the client side is fixed up, of course).
In theory deepen-since and deepen (aka --depth) can be used together to
draw the shallow boundary (whether it's intersection or union is up to
discussion, but if rev-list is used, it's likely intersection). However,
because deepen goes with a custom commit walker, we can't mix the two
yet.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
shallow.c: implement a generic shallow boundary finder based on rev-list
Instead of a custom commit walker like get_shallow_commits(), this new
function uses rev-list to mark NOT_SHALLOW to all reachable commits,
except borders. The definition of reachable is to be defined by the
protocol later. This makes it more flexible to define shallow boundary.
The way we find border is paint all reachable commits NOT_SHALLOW. Any
of them that "touches" commits without NOT_SHALLOW flag are considered
shallow (e.g. zero parents via grafting mechanism). Shallow commits and
their true parents are all marked SHALLOW. Then NOT_SHALLOW is removed
from shallow commits at the end.
There is an interesting observation. With a generic walker, we can
produce all kinds of shallow cutting. In the following graph, every
commit but "x" is reachable. "b" is a parent of "a".
x -- a -- o
/ /
x -- c -- b -- o
After this function is run, "a" and "c" are both considered shallow
commits. After grafting occurs at the client side, what we see is
a -- o
/
c -- b -- o
Notice that because of grafting, "a" has zero parents, so "b" is no
longer a parent of "a".
This is unfortunate and may be solved in two ways. The first is change
the way shallow grafting works and keep "a -- b" connection if "b"
exists and always ends at shallow commits (iow, no loose ends). This is
hard to detect, or at least not cheap to do.
The second way is mark one "x" as shallow commit instead of "a" and
produce this graph at client side:
x -- a -- o
/ /
c -- b -- o
More commits, but simpler grafting rules.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
fetch-pack: use a separate flag for fetch in deepening mode
The shallow repo could be deepened or shortened when then user gives
--depth. But in future that won't be the only way to deepen/shorten a
repo. Stop relying on args->depth in this mode. Future deepening
methods can simply set this flag on instead of updating all these if
expressions.
The new name "deepen" was chosen after the command to define shallow
boundary in pack protocol. New commands also follow this tradition.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
fetch-pack: use a common function for verbose printing
This reduces the number of "if (verbose)" which makes it a bit easier
to read imo. It also makes it easier to redirect all these printouts,
to a file for example.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
upload-pack: make check_non_tip() clean things up on error
On error check_non_tip() will die and not closing file descriptors is no
big deal. The next patch will split the majority of this function out
for reuse in other cases, where die() may not be the only outcome. Same
story for popping SIGPIPE out of the signal chain. So let's make sure we
clean things up properly first.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
upload-pack: move "unshallow" sending code out of deepen()
Also add some more comments in this code because it takes too long to
understand what it does (to me, who should be familiar enough to
understand this code well!)
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
After the last patch, "result" and "backup" are the same. "result" used
to move, but the movement is now contained in send_shallow(). Delete
this redundant variable.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
For now we can handle two types, string and boolean, in
set_helper_option(). Later on we'll add string_list support, which does
not fit well. The new function strbuf_set_helper_option() can be reused
for a separate function that handles string-list.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Failure to bring up httpd for testing is not considered an error, so the
trash directory, which contains this error.log file, is removed and we
don't know what made httpd fail to start. Improve the situation a bit,
print error.log but only in verbose mode.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are two types of string_lists: those that own the
string memory, and those that don't. You can tell the
difference by the strdup_strings flag, and one should use
either STRING_LIST_INIT_DUP, or STRING_LIST_INIT_NODUP as an
initializer.
Historically, the normal all-zeros initialization has
corresponded to the NODUP case. Many sites use no
initializer at all, and that works as a shorthand for that
case. But for a reader of the code, it can be hard to
remember which is which. Let's be more explicit and actually
have each site declare which type it means to use.
This is a fairly mechanical conversion; I assumed each site
was correct as-is, and just switched them all to NODUP.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
blame,shortlog: don't make local option variables static
There's no need for these option variables to be static,
except that they are referenced by the options array itself,
which is static. But having all of this static is simply
unnecessary and confusing (and inconsistent with most other
commands, which either use a static global option list or a
true function-local one).
Note that in some cases we may need to actually initialize
the variables (since we cannot rely on BSS to do so). This
is a net improvement to readability, though, as we can use
the more verbose initializers for our string_lists.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
parse_opt_string_list: stop allocating new strings
The parse_opt_string_list callback is basically a thin
wrapper to string_list_append() any string options we get.
However, it calls:
string_list_append(v, xstrdup(arg));
which duplicates the option value. This is wrong for two
reasons:
1. If the string list has strdup_strings set, then we are
making an extra copy, which is simply leaked.
2. If the string list does not have strdup_strings set,
then we pass memory ownership to the string list, but
it does not realize this. If we later call
string_list_clear(), which can happen if "--no-foo" is
passed, then we will leak all of the existing entries.
Instead, we should just pass the argument straight to
string_list_append, and it can decide whether to copy or not
based on its strdup_strings flag.
It's possible that some (buggy) caller could be relying on
this extra copy (e.g., because it parses some options from
an allocated argv array and then frees the array), but it's
not likely. For one, we generally only use parse_options on
the argv given to us in main(). And two, such a caller is
broken anyway, because other option types like OPT_STRING()
do not make such a copy. This patch brings us in line with
them.
Noticed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 15ffb7cde48 (2011-06-13, submodule update: continue when a checkout
fails), we reasoned it is ok to continue, when there is not much of
a mental burden by the failure. If a recursive submodule fails to clone
because a .gitmodules file is broken (e.g. :
fatal: No url found for submodule path 'foo/bar' in .gitmodules
Failed to recurse into submodule path 'foo', signaled by exit code 128),
this is one of the cases where the user is not expected to have much of
a burden afterwards, so we can also continue in that case.
This means we only want to stop for updating submodules in case of rebase,
merge or custom update command failures, which are all signaled with
exit code 2.
Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Each submodule that is attempted to be cloned, will be retried once in
case of failure after all other submodules were cloned. This helps to
mitigate ephemeral server failures and increases chances of a reliable
clone of a repo with hundreds of submodules immensely.
Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There's continuing work in this area, so clean up unneeded "extern"
keywords rather than schlepping them around. Also split up some overlong
lines and add parameter names in a couple of places.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
lock_ref_sha1_basic(): only handle REF_NODEREF mode
Now lock_ref_sha1_basic() is only called with flags==REF_NODEREF. So we
don't have to handle other cases anymore.
This enables several simplifications, the most interesting of which come
from the fact that ref_lock::orig_ref_name is now always the same as
ref_lock::ref_name:
* Remove ref_lock::orig_ref_name
* Remove local variable orig_refname from lock_ref_sha1_basic()
* ref_name can be initialize once and its value reused
* commit_ref_update() never has to write to the reflog for
lock->orig_ref_name
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
If a transaction includes a non-NODEREF update to a symbolic reference,
we don't have to look it up in lock_ref_for_update(). The reference will
be dereferenced anyway when the split-off update is processed.
This change requires that we store a backpointer from the split-off
update to its parent update, for two reasons:
* We still want to report the original reference name in error messages.
So if an error occurs when checking the split-off update's old_sha1,
walk the parent_update pointers back to find the original reference
name, and report that one.
* We still need to write the old_sha1 of the symref to its reflog. So
after we read the split-off update's reference value, walk the
parent_update pointers back and fill in their old_sha1 fields.
Aside from eliminating unnecessary reads, this change fixes a
subtle (though not very serious) race condition: in the old code, the
old_sha1 of the symref was resolved before the reference that it pointed
at was locked. So it was possible that the old_sha1 value logged to the
symref's reflog could be wrong if another process changed the downstream
reference before it was locked.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Before the previous patch, our first read of the reference happened
before the reference was locked, so we couldn't trust its value and had
to read it again. But now that our first read of the reference happens
after acquiring the lock, there is no need to read it a second time. So
move the read_ref_full() call into the (update->type & REF_ISSYMREF)
block.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Before committing ref updates, split symbolic ref updates into two
parts: an update to the underlying ref, and a log-only update to the
symbolic ref. This ensures that both references are locked correctly
during the transaction, including while their reflogs are updated.
Similarly, if the reference pointed to by HEAD is modified directly, add
a separate log-only update to HEAD, rather than leaving the job of
updating HEAD's reflog to commit_ref_update(). This change ensures that
HEAD is locked correctly while its reflog is being modified, as well as
being cheaper (HEAD only needs to be resolved once).
This makes use of a new function, lock_raw_ref(), which is analogous to
read_raw_ref(), but acquires a lock on the reference before reading it.
This change still has two problems:
* There are redundant read_ref_full() reference lookups.
* It is still possible to get incorrect reflogs for symbolic references
if there is a concurrent update by another process, since the old_oid
of a symref is determined before the lock on the pointed-to ref is
held.
Both problems will soon be fixed.
Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
WIP
ref_transaction_update(): check refname_is_safe() at a minimum
If the user has asked that a new value be set for a reference, we use
check_refname_format() to verify that the reference name satisfies all
of the rules. But in other cases, at least check that refname_is_safe().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Change add_update() to initialize all of the fields in the new
ref_update object. Rename the function to ref_transaction_add_update(),
and increase its visibility to all of the refs-related code.
All of this makes the function more useful for other future callers.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
When renaming refs, don't dereference either the origin or the destination
before renaming.
The origin does not need to be dereferenced because it is presently
forbidden to rename symbolic refs.
Not dereferencing the destination fixes a bug where renaming on top of
a broken symref would use the pointed-to ref name for the moved
reflog.
Add a test for the reflog bug.
Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
The refs infrastructure learns about log-only ref updates, which only
update the reflog. Later, we will use this to separate symbolic
reference resolution from ref updating.
Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
It is nonsensical (and a little bit dangerous) to use REF_ISPRUNING
without REF_NODEREF. Forbid it explicitly. Change the one REF_ISPRUNING
caller to pass REF_NODEREF too.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
lock_ref_sha1_basic(): remove unneeded local variable
resolve_ref_unsafe() can cope with being called with NULL passed to its
flags argument. So lock_ref_sha1_basic() can just hand its own type
parameter through.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>