Re: [ptxdist] [PATCH v2 1/2] doc: dev manual: proof-read and update the "Patching Packages" section

[Roland Hieber <rhi@pengutronix.de>]

On Mon, Sep 02, 2019 at 11:02:00AM +0200, Michael Olbrich wrote:
> On Fri, Aug 30, 2019 at 03:36:37PM +0200, Roland Hieber wrote:
> > Remind the reader to upstream their patches, mention the layer
> > mechanism for search order, fix the logical structure of the whole
> > chapter by removing the superfluous "Creating Patches for a Package"
> > header, mention why `--git` can break packages, and general
> > proof-reading and typo correction.
> > 
> > Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> > ---
> >  doc/dev_manual.rst | 44 ++++++++++++++++++++++++++++----------------
> >  1 file changed, 28 insertions(+), 16 deletions(-)
> > 
> > diff --git a/doc/dev_manual.rst b/doc/dev_manual.rst
> > index d79ebdba780d..5dcad97d4a6c 100644
> > --- a/doc/dev_manual.rst
> > +++ b/doc/dev_manual.rst
> > @@ -494,7 +494,7 @@ At this stage things can fail:
> >  
> >  If the ``configure`` script is not cross compile aware, we are out of
> >  luck. We must patch the source archive in this case to make it work.
> > -Refer to section :ref:`configure_rebuild` on how to use
> > +Refer to the section :ref:`configure_rebuild` on how to use
> >  PTXdist’s features to simplify this task.
> >  If the package depends on external components, these components might
> >  be already part of PTXdist. In this case we just have to add this
> > @@ -1255,15 +1255,27 @@ There can be various reasons why a package must be patched:
> >  
> >  -  or anything else (this case is the most common one)
> >  
> > -PTXdist handles patching automatically. After extracting the archive,
> > -PTXdist checks for the existence of a patch directory with the same name
> > -as the package. If our package’s name is ``foo-1.1.0``, PTXdist searches
> > -for patches in:
> > +Ideally, those problems should be adressed in the original project,
> > +so any patches you add to your BSP or to PTXdist should also be submitted upstream.
> > +The upstream project can often provide better feedback, they can integrate your
> > +patch into a new release, and also maintain your changes as part of the project.
> > +This way we make sure that all advantages of the open source idea work for us;
> > +and your patch can be removed again later when a new release of the project is
> > +integrated into your BSP or into PTXdist.
> > +
> > +PTXdist handles patching automatically.
> > +After extracting the archive of a package, PTXdist checks for the existence of
> > +a patch directory named like its `<PKG>` variable.
> > +Take an exemplary package `foo` with version `1.1.0`:
> > +The variable `FOO` will have the value ``foo-1.1.0``, so PTXdist will look for
> > +a patch directory named ``foo-1.1.0`` in the following locations:
> >  
> >  #. project (``./patches/foo-1.1.0``)
> >  
> >  #. platform (``./configs/|ptxdistPlatformConfigDir|/patches/foo-1.1.0``)
> >  
> > +#. the base layer (see :ref:`layers-in-ptxdist`)
> > +
> 
> Hmmm, there can be zero to N layers. Maybe "all other layers (see ...)"
> 
> >  #. ptxdist (``<ptxdist/installation/path>/patches/foo-1.1.0``)
> >  
> >  The patches from the first location found are used. Note: Due to this
> > @@ -1272,9 +1284,6 @@ PTXdist installation. This can be useful if a project sticks to a
> >  specific PTXdist revision but fixes from a more recent revision of
> >  PTXdist should be used.
> >  
> > -Creating Patches for a Package
> > -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Maybe use this heading above instead of 'Patching Packages' for the whole
> section?

The next section after that also already starts with "Creating...", so
I'd like to prevent repetition here and leave it as-is.

For all other comments, see v3.

 - Roland

> > -
> >  PTXdist uses the utilities *git*, *patch* or *quilt* to work with
> >  patches or patch series. We recommend *git*, as it can manage patch
> >  series in a very easy way.
> > @@ -1331,7 +1340,7 @@ Using Git
> >  """""""""
> >  
> >  Create the patch directory like above for *quilt*,
> > -but only add an empty series file
> > +but only add an empty series file:
> >  
> >  .. code-block:: text
> >  
> > @@ -1352,7 +1361,8 @@ and import the package source as the first commit.
> >    use git to apply patches* in `ptxdist setup` to get this behaviour
> >    as a default for every package.
> >    However, note that this setting is still experimental and can lead to
> 
> Hmm 'experimental' is not correct any more. Can you change this too? Maybe:
> 
> However, note that this is a development feature that can lead to
> unexpected failures - ...
> 
> > -  failures for some packages.
> > +  failures – some packages try to determine if they are being compiled
> > +  from a Git source tree, and behave differently in that case.
> >  
> >  Then you can change into the package build directory
> >  (``platform-<name>/build-target/foo-1.1.0``),
> > @@ -1367,8 +1377,8 @@ The Git history should now look something like this:
> >      * 65a360c2bd60 strfry.c: frobnicate the excusator
> >      * fdc315f6844c (tag: foobar-1.1.0, tag: base) initial commit
> >  
> > -Finally, call ``git ptx-patches`` to regenerate the patch series in the
> > -``patches/foo-1.1.0`` folder.
> > +Finally, call ``git ptx-patches`` to transform those Git commits into the patch
> > +series in the ``patches/foo-1.1.0`` folder.
> >  This way they don't get lost when cleaning the package.
> >  
> >  .. note:: PTXdist will only create a Git repository for packages with
> > @@ -1379,7 +1389,7 @@ This way they don't get lost when cleaning the package.
> >  
> >  Both approaches (Git and quilt) are not suitable for modifying files
> >  that are autogenerated in autotools-based buildsystems.
> > -Refer to section :ref:`configure_rebuild` on how PTXdist can
> > +Refer to the section :ref:`configure_rebuild` on how PTXdist can
> >  handle this special task.
> >  
> >  Adding More Patches to a Package
> > @@ -1393,17 +1403,17 @@ installation directories, NEVER, NEVER, NEVER). Due to the search order
> >  in which PTXdist searches for patches for a specific package, we can
> >  copy the global patch series to our local project directory. Now we have
> >  the permissions to add more patches or modify the existing ones. Also
> > -*quilt* is our friend here to manage the patch series.
> > +*quilt* and *Git* are our friends here to manage the patch series.
> 
> Lowercase 'git' here, I think. We refer to the command-line tool here.
> 
> Michael
> 
> >  
> >  If we think that our new patches are valuable also for others, or they
> >  fix an error, it could be a good idea to send these patches to PTXdist
> > -mainline.
> > +mainline, and to the upstream project too.
> >  
> >  
> >  .. _configure_rebuild:
> >  
> >  Modifying Autotoolized Packages
> > -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >  
> >  Autotoolized packages are very picky when automatically generated files
> >  get patched. The patch order is very important in this case and
> > @@ -1631,6 +1641,8 @@ convenient way to crate simple templates. It is also possible to create
> >  more files. For examples, the builtin ``genimage`` template creates a extra
> >  config file for the new package.
> >  
> > +.. _layers-in-ptxdist:
> > +
> >  Layers in PTXdist
> >  -----------------
> >  
> > -- 
> > 2.23.0
> > 
> > 
> > _______________________________________________
> > ptxdist mailing list
> > ptxdist@pengutronix.de
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de

-- 
Roland Hieber                     | r.hieber@pengutronix.de     |
Pengutronix e.K.                  | https://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim | Phone: +49-5121-206917-5086 |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de