| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The name_rev() function calls itself recursively for each interesting
parent of the commit it got as parameter, and, consequently, it can
segfault when processing a deep history if it exhausts the available
stack space. E.g. running 'git name-rev --all' and 'git name-rev
HEAD~100000' in the gcc, gecko-dev, llvm, and WebKit repositories
results in segfaults on my machine ('ulimit -s' reports 8192kB of
stack size limit), and nowadays the former segfaults in the Linux repo
as well (it reached the necessasry depth sometime between v5.3-rc4 and
-rc5).
Eliminate the recursion by inserting the interesting parents into a
LIFO 'prio_queue' [1] and iterating until the queue becomes empty.
Note that the parent commits must be added in reverse order to the
LIFO 'prio_queue', so their relative order is preserved during
processing, i.e. the first parent should come out first from the
queue, because otherwise performance greatly suffers on mergy
histories [2].
The stacksize-limited test 'name-rev works in a deep repo' in
't6120-describe.sh' demonstrated this issue and expected failure. Now
the recursion is gone, so flip it to expect success. Also gone are
the dmesg entries logging the segfault of that segfaulting 'git
name-rev' process on every execution of the test suite.
Note that this slightly changes the order of lines in the output of
'git name-rev --all', usually swapping two lines every 35 lines in
git.git or every 150 lines in linux.git. This shouldn't matter in
practice, because the output has always been unordered anyway.
This patch is best viewed with '--ignore-all-space'.
[1] Early versions of this patch used a 'commit_list', resulting in
~15% performance penalty for 'git name-rev --all' in 'linux.git',
presumably because of the memory allocation and release for each
insertion and removal. Using a LIFO 'prio_queue' has basically no
effect on performance.
[2] We prefer shorter names, i.e. 'v0.1~234' is preferred over
'v0.1^2~5', meaning that usually following the first parent of a
merge results in the best name for its ancestors. So when later
we follow the remaining parent(s) of a merge, and reach an already
named commit, then we usually find that we can't give that commit
a better name, and thus we don't have to visit any of its
ancestors again.
OTOH, if we were to follow the Nth parent of the merge first, then
the name of all its ancestors would include a corresponding '^N'.
Those are not the best names for those commits, so when later we
reach an already named commit following the first parent of that
merge, then we would have to update the name of that commit and
the names of all of its ancestors as well. Consequently, we would
have to visit many commits several times, resulting in a
significant slowdown.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Following the previous patches in this series we can get the value of
'name_rev()'s 'tip_name' parameter from the 'struct rev_name'
associated with the commit as well.
So let's use 'name->tip_name' instead, which makes the patch
eliminating the recursion of name_rev() a bit easier to follow.
Note that at this point we could drop the 'tip_name' parameter as
well, but that parameter will be necessary later, after the recursion
is eliminated.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Following the previous patches in this series we can get the values of
name_rev()'s 'generation' and 'distance' parameters from the 'stuct
rev_name' associated with the commit as well.
Let's simplify the function's signature and remove these two
unnecessary parameters.
Note that at this point we could do the same with the 'tip_name',
'taggerdate' and 'from_tag' parameters as well, but those parameters
will be necessary later, after the recursion is eliminated.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
At the beginning of the recursive name_rev() function it creates a new
'struct rev_name' instance for each previously unvisited commit or, if
this visit results in better name for an already visited commit, then
updates the 'struct rev_name' instance attached to the commit, or
returns early.
Restructure this so it's caller creates or updates the 'struct
rev_name' instance associated with the commit to be passed as
parameter, i.e. both name_ref() before calling name_rev() and
name_rev() itself as it iterates over the parent commits.
This makes eliminating the recursion a bit easier to follow, and the
condition moved to name_ref() will be moved back to name_rev() after
the recursion is eliminated.
This change also plugs the memory leak that was temporarily unplugged
in the earlier "name-rev: pull out deref handling from the recursion"
patch in this series.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
At the beginning of the recursive name_rev() function it parses the
commit it got as parameter, and returns early if the commit is older
than a cutoff limit.
Restructure this so the caller parses the commit and checks its date,
and doesn't invoke name_rev() if the commit to be passed as parameter
is older than the cutoff, i.e. both name_ref() before calling
name_rev() and name_rev() itself as it iterates over the parent
commits.
This makes eliminating the recursion a bit easier to follow, and the
condition moved to name_ref() will be moved back to name_rev() after
the recursion is eliminated.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The 'if (deref) { ... }' condition near the beginning of the recursive
name_rev() function can only ever be true in the first invocation,
because the 'deref' parameter is always 0 in the subsequent recursive
invocations.
Extract this condition from the recursion into name_rev()'s caller and
drop the function's 'deref' parameter. This makes eliminating the
recursion a bit easier to follow, and it will be moved back into
name_rev() after the recursion is eliminated.
Furthermore, drop the condition that die()s when both 'deref' and
'generation' are non-null (which should have been a BUG() to begin
with).
Note that this change reintroduces the memory leak that was plugged in
in commit 5308224633 (name-rev: avoid leaking memory in the `deref`
case, 2017-05-04), but a later patch (name-rev: restructure
creating/updating 'struct rev_name' instances) in this series will
plug it in again.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In a later patch in this series we'll want to do this in two places.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In 'builtin/name-rev.c' in the name_rev() function there is a loop
iterating over all parents of the given commit, and the loop body
looks like this:
if (parent_number > 1) {
if (generation > 0)
// branch #1
new_name = ...
else
// branch #2
new_name = ...
name_rev(parent, new_name, ...);
} else {
// branch #3
name_rev(...);
}
These conditions are not covered properly in the test suite. As far
as purely test coverage goes, they are all executed several times over
in 't6120-describe.sh'. However, they don't directly influence the
command's output, because the repository used in that test script
contains several branches and tags pointing somewhere into the middle
of the commit DAG, and thus result in a better name for the
to-be-named commit. This can hide bugs: e.g. by replacing the
'new_name' parameter of the first recursive name_rev() call with
'tip_name' (effectively making both branch #1 and #2 a noop) 'git
name-rev --all' shows thousands of bogus names in the Git repository,
but the whole test suite still passes successfully. In an early
version of a later patch in this series I managed to mess up all three
branches (at once!), but the test suite still passed.
So add a new test case that operates on the following history:
A--------------master
\ /
\----------M2
\ /
\---M1-C
\ /
B
and names the commit 'B' to make sure that all three branches are
crucial to determine 'B's name:
- There is only a single ref, so all names are based on 'master',
without any undesired interference from other refs.
- Each time name_rev() follows the second parent of a merge commit,
it appends "^2" to the name. Following 'master's second parent
right at the start ensures that all commits on the ancestry path
from 'master' to 'B' have a different base name from the original
'tip_name' of the very first name_rev() invocation. Currently,
while name_rev() is recursive, it doesn't matter, but it will be
necessary to properly cover all three branches after the recursion
is eliminated later in this series.
- Following 'M2's second parent makes sure that branch #2 (i.e. when
'generation = 0') affects 'B's name.
- Following the only parent of the non-merge commit 'C' ensures that
branch #3 affects 'B's name, and that it increments 'generation'.
- Coming from 'C' 'generation' is 1, thus following 'M1's second
parent makes sure that branch #1 affects 'B's name.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Casting a 'struct object' to 'struct commit' is unnecessary there,
because it's already available in the local 'commit' variable.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
get_name_rev() basically open-codes strip_suffix() before adding a
string to a strbuf.
Let's use the strbuf right from the beginning, i.e. add the whole
string to the strbuf and then use strbuf_strip_suffix(), making the
code more idiomatic.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The 'check_describe' helper function runs 'git describe' outside of
'test_expect_success' blocks, with extra hand-rolled code to record
and examine its exit code.
Update this helper and move the 'git describe' invocation inside the
'test_expect_success' block.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
At the top of 't6120-describe.sh' an ASCII graph illustrates the
repository's history used in this test script. This graph is a bit
misleading, because it swapped the second merge commit's first and
second parents.
When describing/naming a commit it does make a difference which parent
is the first and which is the second/Nth, so update this graph to
accurately represent that second merge.
While at it, move this history graph from the 'test_description'
variable to a regular comment.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Finishing touches to the recent update to the build procedure for
the documentation.
* bc/doc-use-docbook-5:
manpage-bold-literal.xsl: match for namespaced "d:literal" in template
|
|
Regression fix.
* ds/commit-graph-on-fetch:
commit-graph: fix writing first commit-graph during fetch
t5510-fetch.sh: demonstrate fetch.writeCommitGraph bug
|
|
Work-around a lazy fetch glitch.
* jt/delay-fetch-if-missing:
fetch: delay fetch_if_missing=0 until after config
|
|
* https://github.com/prati0100/git-gui:
git-gui: improve Japanese translation
git-gui: add a readme
git-gui: support for diff3 conflict style
git-gui: use existing interface to query a path's attribute
git-gui (Windows): use git-bash.exe if it is available
treewide: correct several "up-to-date" to "up to date"
Fix build with core.autocrlf=true
|
|
l10n-2.24.0-rnd2
* tag 'l10n-2.24.0-rnd2' of https://github.com/git-l10n/git-po:
l10n: zh_CN: for git v2.24.0 l10n round 1~2
l10n: de.po: Update German translation
l10n: sv.po: Update Swedish translation (4695t0f0u)
l10n: bg.po: Updated Bulgarian translation (4694)
l10n: vi(4694t): Updated translation for v2.24.0
l10n: es: 2.24.0 round 2
l10n: it.po: update the Italian translation for Git 2.24.0 round #2
l10n: fr v2.24.0 rnd2
l10n: git.pot: v2.24.0 round 2 (1 new)
l10n: it.po: update the Italian translation for Git 2.24.0
l10n: fr 2.24.0 rnd 1
l10n: git.pot: v2.24.0 round 1 (35 new, 16 removed)
l10n: bg.po: Updated Bulgarian translation (4693)
l10n: sv.po: Update Swedish translation (4674t0f0u)
l10n: Update Catalan translation
|
|
Translate 36 new messages (4694t0f0u) for git 2.24.0.
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
|
|
As per Wikipedia, "In current technical usage, for one to state that a
feature is deprecated is merely a recommendation against using it." It
is thus contradictory to claim that something is not "officially
deprecated" and then to immediately state that we are both discouraging
its use and pointing people elsewhere.
Signed-off-by: Elijah Newren <newren@gmail.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We recently regressed our rendering with Asciidoctor of "literal"
elements in our manpages, i.e, stuff we have placed within `backticks`
in order to render as monospace. In particular, we lost the bold
rendering of such literal text.
The culprit is f6461b82b9 ("Documentation: fix build with Asciidoctor 2",
2019-09-15), where we switched from DocBook 4.5 to DocBook 5 with
Asciidoctor. As part of the switch, we started using the namespaced
DocBook XSLT stylesheets rather than the non-namespaced ones. (See
f6461b82b9 for more details on why we changed to the namespaced ones.)
The bold literals are implemented as an XSLT snippet <xsl:template
match="literal">...</xsl:template>. Now that we use namespaces, this
doesn't pick up our literals like it used to.
Match for "d:literal" in addition to just "literal", after defining the
d namespace. ("d" is what
http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl
uses.) Note that we need to keep matching without the namespace for
AsciiDoc.
This boldness was introduced by 5121a6d993 ("Documentation: option to
render literal text as bold for manpages", 2009-03-27) and made the
default in 5945717009 ("Documentation: bold literals in man",
2016-05-31).
One reason this was not caught in review is that our doc-diff tool diffs
without any boldness, i.e., it "only" compares text. As pointed out by
Peff in review of this patch, one can use `MAN_KEEP_FORMATTING=1
./doc-diff <...>`
This has been optically tested with AsciiDoc 8.6.10, Asciidoctor 1.5.5
and Asciidoctor 2.0.10. I've also verified that doc-diff produces the
empty output for all three programs, as expected, and that with the
MAN_KEEP_FORMATTING trick, AsciiDoc yields no diff, whereas with
Asciidoctor, we get bold literals, just like we want.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Acked-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Matthias Rüster <matthias.ruester@gmail.com>
Reviewed-by: Ralf Thielow <ralf.thielow@gmail.com>
Reviewed-by: Phillip Szelat <phillip.szelat@gmail.com>
|
|
Signed-off-by: Peter Krefting <peter@softwolves.pp.se>
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Comment update.
* wb/fsmonitor-bitmap-fix:
t7519-status-fsmonitor: improve comments
|
|
Fix a rather old bug in gitweb---incremental blame output in
javascript actions mode never worked.
* rl/gitweb-blame-prev-fix:
gitweb: correctly store previous rev in javascript-actions mode
|
|
Fix for a (rather old) buffer-overrun bug.
* js/mingw-needs-hiding-fix:
mingw: avoid a buffer overrun in `needs_hiding()`
|
|
* 'master' of github.com:vnwildman/git:
l10n: vi(4694t): Updated translation for v2.24.0
|
|
* 'next' of github.com:ChrisADR/git-po:
l10n: es: 2.24.0 round 2
|
|
The comments for the staging/unstaging test did not accurately
describe the scenario being tested. It is not essential that
the test files being staged/unstaged appear at the end of the
index. All that is required is that the test files are not
flagged with CE_FSMONITOR_VALID and have a position in the
index greater than the number of entries in the index after
unstaging.
The comment for this test has been updated to be more
accurate with respect to the scenario that's being tested.
Signed-off-by: William Baker <William.Baker@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Alexander Shopov <ash@kambanaria.org>
|
|
Signed-off-by: Tran Ngoc Quan <vnwildman@gmail.com>
|
|
Signed-off-by: Christopher Diaz Riveros <christopher.diaz.riv@gmail.com>
|
|
* 'update-italian-translation' of github.com:AlessandroMenti/git-po:
l10n: it.po: update the Italian translation for Git 2.24.0 round #2
|
|
Signed-off-by: Alessandro Menti <alessandro.menti@alessandromenti.it>
|
|
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
|
|
Generate po/git.pot from v2.24.0-rc1 for git v2.24.0 l10n round 2.
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
|
|
Git 2.24-rc1
* tag 'v2.24.0-rc1' of github.com:git/git:
Git 2.24-rc1
repo-settings: read an int for index.version
ci: fix GCC install in the Travis CI GCC OSX job
Eleventh batch
ci(osx): use new location of the `perforce` cask
t7419: change test_must_fail to ! for grep
t4014: make output-directory tests self-contained
ci(visual-studio): actually run the tests in parallel
ci(visual-studio): use strict compile flags, and optimization
userdiff: fix some corner cases in dts regex
test-progress: fix test failures on big-endian systems
completion: clarify installation instruction for zsh
grep: avoid leak of chartables in PCRE2
grep: make PCRE2 aware of custom allocator
grep: make PCRE1 aware of custom allocator
remote-curl: pass on atomic capability to remote side
diff-highlight: fix a whitespace nit
fsmonitor: don't fill bitmap with entries to be removed
|
|
When this function is passed a path with a trailing slash, it runs right
over the end of that path.
Let's fix this.
Co-authored-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Without this change, the setting
$feature{'javascript-actions'}{'default'} = [1];
in gitweb.conf breaks gitweb's blame page: clicking on line numbers
displayed in the second column on the page has no effect.
For comparison, with javascript-actions disabled, clicking on line
numbers loads the previous version of the line.
Addresses https://bugs.debian.org/741883.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Robert Luberda <robert@debian.org>
Acked-by: Jakub Narębski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The previous commit includes a failing test for an issue around
fetch.writeCommitGraph and fetching in a repo with a submodule. Here, we
fix that bug and set the test to "test_expect_success".
The problem arises with this set of commands when the remote repo at
<url> has a submodule. Note that --recurse-submodules is not needed to
demonstrate the bug.
$ git clone <url> test
$ cd test
$ git -c fetch.writeCommitGraph=true fetch origin
Computing commit graph generation numbers: 100% (12/12), done.
BUG: commit-graph.c:886: missing parent <hash1> for commit <hash2>
Aborted (core dumped)
As an initial fix, I converted the code in builtin/fetch.c that calls
write_commit_graph_reachable() to instead launch a "git commit-graph
write --reachable --split" process. That code worked, but is not how we
want the feature to work long-term.
That test did demonstrate that the issue must be something to do with
internal state of the 'git fetch' process.
The write_commit_graph() method in commit-graph.c ensures the commits we
plan to write are "closed under reachability" using close_reachable().
This method walks from the input commits, and uses the UNINTERESTING
flag to mark which commits have already been visited. This allows the
walk to take O(N) time, where N is the number of commits, instead of
O(P) time, where P is the number of paths. (The number of paths can be
exponential in the number of commits.)
However, the UNINTERESTING flag is used in lots of places in the
codebase. This flag usually means some barrier to stop a commit walk,
such as in revision-walking to compare histories. It is not often
cleared after the walk completes because the starting points of those
walks do not have the UNINTERESTING flag, and clear_commit_marks() would
stop immediately.
This is happening during a 'git fetch' call with a remote. The fetch
negotiation is comparing the remote refs with the local refs and marking
some commits as UNINTERESTING.
I tested running clear_commit_marks_many() to clear the UNINTERESTING
flag inside close_reachable(), but the tips did not have the flag, so
that did nothing.
It turns out that the calculate_changed_submodule_paths() method is at
fault. Thanks, Peff, for pointing out this detail! More specifically,
for each submodule, the collect_changed_submodules() runs a revision
walk to essentially do file-history on the list of submodules. That
revision walk marks commits UNININTERESTING if they are simplified away
by not changing the submodule.
Instead, I finally arrived on the conclusion that I should use a flag
that is not used in any other part of the code. In commit-reach.c, a
number of flags were defined for commit walk algorithms. The REACHABLE
flag seemed like it made the most sense, and it seems it was not
actually used in the file. The REACHABLE flag was used in early versions
of commit-reach.c, but was removed by 4fbcca4 (commit-reach: make
can_all_from_reach... linear, 2018-07-20).
Add the REACHABLE flag to commit-graph.c and use it instead of
UNINTERESTING in close_reachable(). This fixes the bug in manual
testing.
Reported-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Szeder Gábor <szeder.dev@gmail.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
While dogfooding, Johannes found a bug in the fetch.writeCommitGraph
config behavior. His example initially happened during a clone with
--recurse-submodules, we found that this happens with the first fetch
after cloning a repository that contains a submodule:
$ git clone <url> test
$ cd test
$ git -c fetch.writeCommitGraph=true fetch origin
Computing commit graph generation numbers: 100% (12/12), done.
BUG: commit-graph.c:886: missing parent <hash1> for commit <hash2>
Aborted (core dumped)
In the repo I had cloned, there were really 60 commits to scan, but
only 12 were in the list to write when calling
compute_generation_numbers(). A commit in the list expects to see a
parent, but that parent is not in the list.
A follow-up will fix the bug, but first we create a test that
demonstrates the problem. This test must be careful about an existing
commit-graph file, since GIT_TEST_COMMIT_GRAPH=1 will cause the repo we
are cloning to already have one. This then prevents the incremtnal
commit-graph write during the first 'git fetch'.
Helped-by: Jeff King <peff@peff.net>
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Helped-by: Szeder Gábor <szeder.dev@gmail.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
CI build fix.
* sg/ci-osx-gcc8-fix:
ci: fix GCC install in the Travis CI GCC OSX job
|
|
The codepath that reads the index.version configuration was broken
with a recent update, which has been corrected.
* ds/feature-macros:
repo-settings: read an int for index.version
|
|
Update installation procedure for Perforce on MacOS in the CI jobs
running on Azure pipelines, which was failing.
* js/azure-ci-osx-fix:
ci(osx): use new location of the `perforce` cask
|
|
Test update.
* bw/format-patch-o-create-leading-dirs:
t4014: make output-directory tests self-contained
|
|
Test update.
* dl/submodule-set-branch:
t7419: change test_must_fail to ! for grep
|
SZEDER Gábor
René Scharfe
Junio C Hamano
Jiang Xin
Elijah Newren
Martin Ågren
Matthias Rüster
Peter Krefting
William Baker
Alexander Shopov
Tran Ngoc Quan
Christopher Diaz Riveros
Alessandro Menti
Jean-Noël Avila
Johannes Schindelin
Robert Luberda
Derrick Stolee