Skip to content

Commit

Permalink
Merge tag 'docs-5.18-2' of git://git.lwn.net/linux
Browse files Browse the repository at this point in the history
Pull more documentation updates from Jonathan Corbet:
 "Some late-arriving documentation improvements.

  This is mostly build-system fixes from Mauro and Akira; I also took
  the liberty of dropping in my 'messy diffstat' document"

* tag 'docs-5.18-2' of git://git.lwn.net/linux:
  docs: Add a document on how to fix a messy diffstat
  docs: sphinx/requirements: Limit jinja2<3.1
  Documentation: kunit: Fix cross-referencing warnings
  scripts/kernel-doc: change the line number meta info
  scripts/get_abi: change the file/line number meta info
  docs: kernel_include.py: add sphinx build dependencies
  docs: kernel_abi.py: add sphinx build dependencies
  docs: kernel_feat.py: add build dependencies
  scripts/get_feat.pl: allow output the parsed file names
  docs: kfigure.py: Don't warn of missing PDF converter in 'make htmldocs'
  Documentation: Fix duplicate statement about raw_spinlock_t type
  • Loading branch information
torvalds committed Mar 31, 2022
2 parents b8321ed + 022bb49 commit b4a5ea0
Show file tree
Hide file tree
Showing 13 changed files with 148 additions and 25 deletions.
13 changes: 2 additions & 11 deletions Documentation/dev-tools/kunit/architecture.rst
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,7 @@ The fundamental unit in KUnit is the test case. The KUnit test cases are
grouped into KUnit suites. A KUnit test case is a function with type
signature ``void (*)(struct kunit *test)``.
These test case functions are wrapped in a struct called
``struct kunit_case``. For code, see:

.. kernel-doc:: include/kunit/test.h
:identifiers: kunit_case
struct kunit_case.

.. note:
``generate_params`` is optional for non-parameterized tests.
Expand Down Expand Up @@ -152,18 +149,12 @@ Parameterized Tests
Each KUnit parameterized test is associated with a collection of
parameters. The test is invoked multiple times, once for each parameter
value and the parameter is stored in the ``param_value`` field.
The test case includes a ``KUNIT_CASE_PARAM()`` macro that accepts a
The test case includes a KUNIT_CASE_PARAM() macro that accepts a
generator function.
The generator function is passed the previous parameter and returns the next
parameter. It also provides a macro to generate common-case generators based on
arrays.

For code, see:

.. kernel-doc:: include/kunit/test.h
:identifiers: KUNIT_ARRAY_PARAM


kunit_tool (Command Line Test Harness)
======================================

Expand Down
3 changes: 0 additions & 3 deletions Documentation/locking/locktypes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,6 @@ raw_spinlock_t and spinlock_t
raw_spinlock_t
--------------

raw_spinlock_t is a strict spinning lock implementation regardless of the
kernel configuration including PREEMPT_RT enabled kernels.

raw_spinlock_t is a strict spinning lock implementation in all kernels,
including PREEMPT_RT kernels. Use raw_spinlock_t only in real critical
core code, low-level interrupt handling and places where disabling
Expand Down
1 change: 1 addition & 0 deletions Documentation/maintainer/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ additions to this manual.
configure-git
rebasing-and-merging
pull-requests
messy-diffstat
maintainer-entry-profile
modifying-patches

96 changes: 96 additions & 0 deletions Documentation/maintainer/messy-diffstat.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
.. SPDX-License-Identifier: GPL-2.0
=====================================
Handling messy pull-request diffstats
=====================================

Subsystem maintainers routinely use ``git request-pull`` as part of the
process of sending work upstream. Normally, the result includes a nice
diffstat that shows which files will be touched and how much of each will
be changed. Occasionally, though, a repository with a relatively
complicated development history will yield a massive diffstat containing a
great deal of unrelated work. The result looks ugly and obscures what the
pull request is actually doing. This document describes what is happening
and how to fix things up; it is derived from The Wisdom of Linus Torvalds,
found in Linus1_ and Linus2_.

.. _Linus1: https://lore.kernel.org/lkml/CAHk-=wg3wXH2JNxkQi+eLZkpuxqV+wPiHhw_Jf7ViH33Sw7PHA@mail.gmail.com/
.. _Linus2: https://lore.kernel.org/lkml/CAHk-=wgXbSa8yq8Dht8at+gxb_idnJ7X5qWZQWRBN4_CUPr=eQ@mail.gmail.com/

A Git development history proceeds as a series of commits. In a simplified
manner, mainline kernel development looks like this::

... vM --- vN-rc1 --- vN-rc2 --- vN-rc3 --- ... --- vN-rc7 --- vN

If one wants to see what has changed between two points, a command like
this will do the job::

$ git diff --stat --summary vN-rc2..vN-rc3

Here, there are two clear points in the history; Git will essentially
"subtract" the beginning point from the end point and display the resulting
differences. The requested operation is unambiguous and easy enough to
understand.

When a subsystem maintainer creates a branch and commits changes to it, the
result in the simplest case is a history that looks like::

... vM --- vN-rc1 --- vN-rc2 --- vN-rc3 --- ... --- vN-rc7 --- vN
|
+-- c1 --- c2 --- ... --- cN

If that maintainer now uses ``git diff`` to see what has changed between
the mainline branch (let's call it "linus") and cN, there are still two
clear endpoints, and the result is as expected. So a pull request
generated with ``git request-pull`` will also be as expected. But now
consider a slightly more complex development history::

... vM --- vN-rc1 --- vN-rc2 --- vN-rc3 --- ... --- vN-rc7 --- vN
| |
| +-- c1 --- c2 --- ... --- cN
| /
+-- x1 --- x2 --- x3

Our maintainer has created one branch at vN-rc1 and another at vN-rc2; the
two were then subsequently merged into c2. Now a pull request generated
for cN may end up being messy indeed, and developers often end up wondering
why.

What is happening here is that there are no longer two clear end points for
the ``git diff`` operation to use. The development culminating in cN
started in two different places; to generate the diffstat, ``git diff``
ends up having pick one of them and hoping for the best. If the diffstat
starts at vN-rc1, it may end up including all of the changes between there
and the second origin end point (vN-rc2), which is certainly not what our
maintainer had in mind. With all of that extra junk in the diffstat, it
may be impossible to tell what actually happened in the changes leading up
to cN.

Maintainers often try to resolve this problem by, for example, rebasing the
branch or performing another merge with the linus branch, then recreating
the pull request. This approach tends not to lead to joy at the receiving
end of that pull request; rebasing and/or merging just before pushing
upstream is a well-known way to get a grumpy response.

So what is to be done? The best response when confronted with this
situation is to indeed to do a merge with the branch you intend your work
to be pulled into, but to do it privately, as if it were the source of
shame. Create a new, throwaway branch and do the merge there::

... vM --- vN-rc1 --- vN-rc2 --- vN-rc3 --- ... --- vN-rc7 --- vN
| | |
| +-- c1 --- c2 --- ... --- cN |
| / | |
+-- x1 --- x2 --- x3 +------------+-- TEMP

The merge operation resolves all of the complications resulting from the
multiple beginning points, yielding a coherent result that contains only
the differences from the mainline branch. Now it will be possible to
generate a diffstat with the desired information::

$ git diff -C --stat --summary linus..TEMP

Save the output from this command, then simply delete the TEMP branch;
definitely do not expose it to the outside world. Take the saved diffstat
output and edit it into the messy pull request, yielding a result that
shows what is really going on. That request can then be sent upstream.
6 changes: 5 additions & 1 deletion Documentation/sphinx/kernel_abi.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ def runCmd(self, cmd, **kwargs):
return out

def nestedParse(self, lines, fname):
env = self.state.document.settings.env
content = ViewList()
node = nodes.section()

Expand All @@ -137,7 +138,7 @@ def nestedParse(self, lines, fname):
code_block += "\n " + l
lines = code_block + "\n\n"

line_regex = re.compile("^#define LINENO (\S+)\#([0-9]+)$")
line_regex = re.compile("^\.\. LINENO (\S+)\#([0-9]+)$")
ln = 0
n = 0
f = fname
Expand All @@ -154,6 +155,9 @@ def nestedParse(self, lines, fname):
self.do_parse(content, node)
content = ViewList()

# Add the file to Sphinx build dependencies
env.note_dependency(os.path.abspath(f))

f = new_f

# sphinx counts lines from 0
Expand Down
20 changes: 18 additions & 2 deletions Documentation/sphinx/kernel_feat.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import codecs
import os
import re
import subprocess
import sys

Expand Down Expand Up @@ -82,7 +83,7 @@ def run(self):

env = doc.settings.env
cwd = path.dirname(doc.current_source)
cmd = "get_feat.pl rest --dir "
cmd = "get_feat.pl rest --enable-fname --dir "
cmd += self.arguments[0]

if len(self.arguments) > 1:
Expand All @@ -102,7 +103,22 @@ def run(self):
shell_env["srctree"] = srctree

lines = self.runCmd(cmd, shell=True, cwd=cwd, env=shell_env)
nodeList = self.nestedParse(lines, fname)

line_regex = re.compile("^\.\. FILE (\S+)$")

out_lines = ""

for line in lines.split("\n"):
match = line_regex.search(line)
if match:
fname = match.group(1)

# Add the file to Sphinx build dependencies
env.note_dependency(os.path.abspath(fname))
else:
out_lines += line + "\n"

nodeList = self.nestedParse(out_lines, fname)
return nodeList

def runCmd(self, cmd, **kwargs):
Expand Down
3 changes: 3 additions & 0 deletions Documentation/sphinx/kernel_include.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class KernelInclude(Include):
u"""KernelInclude (``kernel-include``) directive"""

def run(self):
env = self.state.document.settings.env
path = os.path.realpath(
os.path.expandvars(self.arguments[0]))

Expand All @@ -70,6 +71,8 @@ def run(self):

self.arguments[0] = path

env.note_dependency(os.path.abspath(path))

#return super(KernelInclude, self).run() # won't work, see HINTs in _run()
return self._run()

Expand Down
2 changes: 1 addition & 1 deletion Documentation/sphinx/kerneldoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def run(self):
result = ViewList()

lineoffset = 0;
line_regex = re.compile("^#define LINENO ([0-9]+)$")
line_regex = re.compile("^\.\. LINENO ([0-9]+)$")
for line in lines:
match = line_regex.search(line)
if match:
Expand Down
8 changes: 5 additions & 3 deletions Documentation/sphinx/kfigure.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ def setupTools(app):
if convert_cmd:
kernellog.verbose(app, "use convert(1) from: " + convert_cmd)
else:
kernellog.warn(app,
kernellog.verbose(app,
"Neither inkscape(1) nor convert(1) found.\n"
"For SVG to PDF conversion, "
"install either Inkscape (https://inkscape.org/) (preferred) or\n"
Expand Down Expand Up @@ -296,8 +296,10 @@ def convert_image(img_node, translator, src_fname=None):

if translator.builder.format == 'latex':
if not inkscape_cmd and convert_cmd is None:
kernellog.verbose(app,
"no SVG to PDF conversion available / include SVG raw.")
kernellog.warn(app,
"no SVG to PDF conversion available / include SVG raw."
"\nIncluding large raw SVGs can cause xelatex error."
"\nInstall Inkscape (preferred) or ImageMagick.")
img_node.replace_self(file2literal(src_fname))
else:
dst_fname = path.join(translator.builder.outdir, fname + '.pdf')
Expand Down
2 changes: 2 additions & 0 deletions Documentation/sphinx/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
# jinja2>=3.1 is not compatible with Sphinx<4.0
jinja2<3.1
sphinx_rtd_theme
Sphinx==2.4.4
4 changes: 2 additions & 2 deletions scripts/get_abi.pl
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ sub output_rest {
my @filepath = split / /, $data{$what}->{filepath};

if ($enable_lineno) {
printf "#define LINENO %s%s#%s\n\n",
printf ".. LINENO %s%s#%s\n\n",
$prefix, $file[0],
$data{$what}->{line_no};
}
Expand Down Expand Up @@ -1023,7 +1023,7 @@ =head1 OPTIONS
=item B<--enable-lineno>
Enable output of #define LINENO lines.
Enable output of .. LINENO lines.
=item B<--debug> I<debug level>
Expand Down
11 changes: 11 additions & 0 deletions scripts/get_feat.pl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
my $debug;
my $arch;
my $feat;
my $enable_fname;

my $basename = abs_path($0);
$basename =~ s,/[^/]+$,/,;
Expand All @@ -31,6 +32,7 @@
'arch=s' => \$arch,
'feat=s' => \$feat,
'feature=s' => \$feat,
"enable-fname" => \$enable_fname,
man => \$man
) or pod2usage(2);

Expand Down Expand Up @@ -95,6 +97,10 @@ sub parse_feat {
return if ($file =~ m,($prefix)/arch-support.txt,);
return if (!($file =~ m,arch-support.txt$,));

if ($enable_fname) {
printf ".. FILE %s\n", abs_path($file);
}

my $subsys = "";
$subsys = $2 if ( m,.*($prefix)/([^/]+).*,);

Expand Down Expand Up @@ -580,6 +586,11 @@ =head1 OPTIONS
Changes the location of the Feature files. By default, it uses
the Documentation/features directory.
=item B<--enable-fname>
Prints the file name of the feature files. This can be used in order to
track dependencies during documentation build.
=item B<--debug>
Put the script in verbose mode, useful for debugging. Can be called multiple
Expand Down
4 changes: 2 additions & 2 deletions scripts/kernel-doc
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ sub get_kernel_version() {
sub print_lineno {
my $lineno = shift;
if ($enable_lineno && defined($lineno)) {
print "#define LINENO " . $lineno . "\n";
print ".. LINENO " . $lineno . "\n";
}
}
##
Expand Down Expand Up @@ -2478,7 +2478,7 @@ May be specified multiple times.
=item -enable-lineno
Enable output of #define LINENO lines.
Enable output of .. LINENO lines.
=back
Expand Down

0 comments on commit b4a5ea0

Please sign in to comment.