[pbs-devel] [PATCH proxmox-backup 1/2] docs: fix references to changed refs

[pbs-devel] [PATCH proxmox-backup 1/2] docs: fix references to changed refs

[Aaron Lauterer]

With commit ec1ae7e63180768c9ad8fa52d437a1cee6ef1c50 some refs were
changed by getting prefixes and such. We need to adapt the places that
reference them as well

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 docs/backup-client.rst      | 2 +-
 docs/gui.rst                | 2 +-
 docs/introduction.rst       | 2 +-
 docs/maintenance.rst        | 6 +++---
 docs/managing-remotes.rst   | 2 +-
 docs/pxar/description.rst   | 2 +-
 docs/storage.rst            | 4 ++--
 docs/technical-overview.rst | 4 ++--
 8 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/docs/backup-client.rst b/docs/backup-client.rst
index 800ee43f..415e5bf0 100644
--- a/docs/backup-client.rst
+++ b/docs/backup-client.rst
@@ -657,7 +657,7 @@ shows the list of existing snapshots and what actions prune would take.
 
 .. note:: Neither the ``prune`` command nor the ``forget`` command free space
    in the chunk-store. The chunk-store still contains the data blocks. To free
-   space you need to perform :ref:`garbage-collection`.
+   space you need to perform :ref:`client_garbage-collection`.
 
 
 .. _client_garbage-collection:
diff --git a/docs/gui.rst b/docs/gui.rst
index b2b1fb73..719675e9 100644
--- a/docs/gui.rst
+++ b/docs/gui.rst
@@ -129,7 +129,7 @@ top panel to view:
 * **Content**: Information on the datastore's backup groups and their respective
   contents
 * **Prune & GC**: Schedule :ref:`pruning <backup-pruning>` and :ref:`garbage
-  collection <garbage-collection>` operations, and run garbage collection
+  collection <client_garbage-collection>` operations, and run garbage collection
   manually
 * **Sync Jobs**: Create, manage and run :ref:`syncjobs` from remote servers
 * **Verify Jobs**: Create, manage and run :ref:`maintenance_verification` jobs on the
diff --git a/docs/introduction.rst b/docs/introduction.rst
index 8545bf2c..5e754477 100644
--- a/docs/introduction.rst
+++ b/docs/introduction.rst
@@ -15,7 +15,7 @@ encryption (AE_). Using :term:`Rust` as the implementation language guarantees h
 performance, low resource usage, and a safe, high-quality codebase.
 
 Proxmox Backup uses state of the art cryptography for both client-server
-communication and backup content :ref:`encryption <encryption>`. All
+communication and backup content :ref:`encryption <client_encryption>`. All
 client-server communication uses `TLS
 <https://en.wikipedia.org/wiki/Transport_Layer_Security>`_, and backup data can
 be encrypted on the client-side before sending, making it safer to back up data
diff --git a/docs/maintenance.rst b/docs/maintenance.rst
index 561b4fe4..c2771e36 100644
--- a/docs/maintenance.rst
+++ b/docs/maintenance.rst
@@ -118,11 +118,11 @@ high, but you cannot recreate backup snapshots from the past.
 Garbage Collection
 ------------------
 
-You can monitor and run :ref:`garbage collection <garbage-collection>` on the
+You can monitor and run :ref:`garbage collection <client_garbage-collection>` on the
 Proxmox Backup Server using the ``garbage-collection`` subcommand of
 ``proxmox-backup-manager``. You can use the ``start`` subcommand to manually
 start garbage collection on an entire datastore and the ``status`` subcommand to
-see attributes relating to the :ref:`garbage collection <garbage-collection>`.
+see attributes relating to the :ref:`garbage collection <client_garbage-collection>`.
 
 This functionality can also be accessed in the GUI, by navigating to **Prune &
 GC** from the top panel. From here, you can edit the schedule at which garbage
@@ -142,7 +142,7 @@ Verification
 Proxmox Backup offers various verification options to ensure that backup data is
 intact.  Verification is generally carried out through the creation of verify
 jobs. These are scheduled tasks that run verification at a given interval (see
-:ref:`calendar-events`). With these, you can set whether already verified
+:ref:`calendar-event-scheduling`). With these, you can set whether already verified
 snapshots are ignored, as well as set a time period, after which verified jobs
 are checked again. The interface for creating verify jobs can be found under the
 **Verify Jobs** tab of the datastore.
diff --git a/docs/managing-remotes.rst b/docs/managing-remotes.rst
index e29ae37f..88ab3ba2 100644
--- a/docs/managing-remotes.rst
+++ b/docs/managing-remotes.rst
@@ -65,7 +65,7 @@ the ``proxmox-backup-manager sync-job`` command.  The configuration information
 for sync jobs is stored at ``/etc/proxmox-backup/sync.cfg``. To create a new
 sync job, click the add button in the GUI, or use the ``create`` subcommand.
 After creating a sync job, you can either start it manually from the GUI or
-provide it with a schedule (see :ref:`calendar-events`) to run regularly.
+provide it with a schedule (see :ref:`calendar-event-scheduling`) to run regularly.
 
 .. code-block:: console
 
diff --git a/docs/pxar/description.rst b/docs/pxar/description.rst
index 18be7687..be78623d 100644
--- a/docs/pxar/description.rst
+++ b/docs/pxar/description.rst
@@ -80,7 +80,7 @@ These files must contain one pattern per line, again later patterns win over
 previous ones.
 The patterns control file exclusions of files present within the given directory
 or further below it in the tree.
-The behavior is the same as described in :ref:`creating-backups`.
+The behavior is the same as described in :ref:`client_creating_backups`.
 
 Extracting an Archive
 ^^^^^^^^^^^^^^^^^^^^^
diff --git a/docs/storage.rst b/docs/storage.rst
index 45807b21..de3f0f46 100644
--- a/docs/storage.rst
+++ b/docs/storage.rst
@@ -119,8 +119,8 @@ directory on the filesystem. Each datastore also has associated retention
 settings of how many backup snapshots for each interval of ``hourly``,
 ``daily``, ``weekly``, ``monthly``, ``yearly`` as well as a time-independent
 number of backups to keep in that store. :ref:`backup-pruning` and
-:ref:`garbage collection <garbage-collection>` can also be configured to run
-periodically based on a configured schedule (see :ref:`calendar-events`) per datastore.
+:ref:`garbage collection <client_garbage-collection>` can also be configured to run
+periodically based on a configured schedule (see :ref:`calendar-event-scheduling`) per datastore.
 
 
 .. _storage_datastore_create:
diff --git a/docs/technical-overview.rst b/docs/technical-overview.rst
index 9dba2557..1bf59e23 100644
--- a/docs/technical-overview.rst
+++ b/docs/technical-overview.rst
@@ -7,8 +7,8 @@ Datastores
 ----------
 
 A Datastore is the logical place where :ref:`Backup Snapshots
-<backup_snapshot>` and their chunks are stored. Snapshots consist of a
-manifest, blobs, dynamic- and fixed-indexes (see :ref:`terminology`), and are
+<term_backup_snapshot>` and their chunks are stored. Snapshots consist of a
+manifest, blobs, dynamic- and fixed-indexes (see :ref:`terms`), and are
 stored in the following directory structure:
 
  <datastore-root>/<type>/<id>/<time>/
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 2/2] docs/scanrefs: fix handling if ref is same as headline

[Aaron Lauterer]

If the ref is named the same as the headline (once normalized), sphinx
will return a 'idX' value in node['ids'][1] which we use for the label
ID. The headline is always present at index 0.

Checking for that and using index 0 in case we do get a 'idX' helps us
to avoid using the 'idX' as keys in our OnlineHelpInfo.js and actually
use the intended key.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 docs/_ext/proxmox-scanrefs.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/docs/_ext/proxmox-scanrefs.py b/docs/_ext/proxmox-scanrefs.py
index 1b3c0615..0d626561 100644
--- a/docs/_ext/proxmox-scanrefs.py
+++ b/docs/_ext/proxmox-scanrefs.py
@@ -90,7 +90,18 @@ class ReflabelMapper(Builder):
                 if hasattr(node, 'expect_referenced_by_id') and len(node['ids']) > 1: # explicit labels
                     filename = self.env.doc2path(docname)
                     filename_html = re.sub('.rst', '.html', filename)
-                    labelid = node['ids'][1] # [0] is predefined by sphinx, we need [1] for explicit ones
+
+                    # node['ids'][0] contains a normalized version of the
+                    # headline.  If the ref and headline are the same
+                    # (normalized) sphinx will set the node['ids'][1] to a
+                    # generic id in the format `idX` where X is numeric. If the
+                    # ref and headline are not the same, the ref name will be
+                    # stored in node['ids'][1]
+                    if re.match('^id[0-9]*$', node['ids'][1]):
+                        labelid = node['ids'][0]
+                    else:
+                        labelid = node['ids'][1]
+
                     title = cast(nodes.title, node[0])
                     logger.info('traversing section {}'.format(title.astext()))
                     ref_name = getattr(title, 'rawsource', title.astext())
-- 
2.20.1

[pbs-devel] applied: [PATCH proxmox-backup 1/2] docs: fix references to changed refs

[Dietmar Maurer]

applied both patches

Re: [pbs-devel] [PATCH proxmox-backup 1/2] docs: fix references to changed refs

[Thomas Lamprecht]

On 05.02.21 16:10, Aaron Lauterer wrote:
> With commit ec1ae7e63180768c9ad8fa52d437a1cee6ef1c50 some refs were
> changed by getting prefixes and such. We need to adapt the places that
> reference them as well

The commit messages forgets to mention that underlying issue here is that
the build still happily succeeds, which it really must not.

sphinx-builder can make treat warnings as errors, but there are so many
(especially from generated man pages and such) that this is unpractical.

What we actually can do is listening to the missing reference event and
abort the build our self:

https://www.sphinx-doc.org/de/latest/extdev/appapi.html#event-missing-reference

Re: [pbs-devel] [PATCH proxmox-backup 2/2] docs/scanrefs: fix handling if ref is same as headline

[Thomas Lamprecht]

On 05.02.21 16:10, Aaron Lauterer wrote:
> If the ref is named the same as the headline (once normalized), sphinx
> will return a 'idX' value in node['ids'][1] which we use for the label
> ID. The headline is always present at index 0.
> 
> Checking for that and using index 0 in case we do get a 'idX' helps us
> to avoid using the 'idX' as keys in our OnlineHelpInfo.js and actually
> use the intended key.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  docs/_ext/proxmox-scanrefs.py | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/_ext/proxmox-scanrefs.py b/docs/_ext/proxmox-scanrefs.py
> index 1b3c0615..0d626561 100644
> --- a/docs/_ext/proxmox-scanrefs.py
> +++ b/docs/_ext/proxmox-scanrefs.py
> @@ -90,7 +90,18 @@ class ReflabelMapper(Builder):
>                  if hasattr(node, 'expect_referenced_by_id') and len(node['ids']) > 1: # explicit labels
>                      filename = self.env.doc2path(docname)
>                      filename_html = re.sub('.rst', '.html', filename)
> -                    labelid = node['ids'][1] # [0] is predefined by sphinx, we need [1] for explicit ones
> +
> +                    # node['ids'][0] contains a normalized version of the
> +                    # headline.  If the ref and headline are the same
> +                    # (normalized) sphinx will set the node['ids'][1] to a
> +                    # generic id in the format `idX` where X is numeric. If the
> +                    # ref and headline are not the same, the ref name will be
> +                    # stored in node['ids'][1]

can you point me from where you derived that?

Because I think there are always two refs in such cases where we set one
above a heading: the implicit heading one and the explicit from us.
The always get normalized, but the implicit has a fallback if there's a ref
conflict with an explicit or even another implicit one, when a title is
reused in the same chapter or so?

Do we also have access to the chapter id/name here?
Then we could enforce that explicit ones must have that prefixed.

> +                    if re.match('^id[0-9]*$', node['ids'][1]):

should be a + not * op? we want to avoid clashes with real possible refs
as much as possible..

What happens if I set now one to id1 and there would be already an id1?

I just really do not want to revisit this again, and loosing references
is a no-go, the docs must work.

> +                        labelid = node['ids'][0]
> +                    else:
> +                        labelid = node['ids'][1]
> +
>                      title = cast(nodes.title, node[0])
>                      logger.info('traversing section {}'.format(title.astext()))
>                      ref_name = getattr(title, 'rawsource', title.astext())
> 

Re: [pbs-devel] [PATCH proxmox-backup 2/2] docs/scanrefs: fix handling if ref is same as headline

[Aaron Lauterer]

On 2/6/21 9:22 AM, Thomas Lamprecht wrote:
> On 05.02.21 16:10, Aaron Lauterer wrote:
>> If the ref is named the same as the headline (once normalized), sphinx
>> will return a 'idX' value in node['ids'][1] which we use for the label
>> ID. The headline is always present at index 0.
>>
>> Checking for that and using index 0 in case we do get a 'idX' helps us
>> to avoid using the 'idX' as keys in our OnlineHelpInfo.js and actually
>> use the intended key.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>>   docs/_ext/proxmox-scanrefs.py | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/_ext/proxmox-scanrefs.py b/docs/_ext/proxmox-scanrefs.py
>> index 1b3c0615..0d626561 100644
>> --- a/docs/_ext/proxmox-scanrefs.py
>> +++ b/docs/_ext/proxmox-scanrefs.py
>> @@ -90,7 +90,18 @@ class ReflabelMapper(Builder):
>>                   if hasattr(node, 'expect_referenced_by_id') and len(node['ids']) > 1: # explicit labels
>>                       filename = self.env.doc2path(docname)
>>                       filename_html = re.sub('.rst', '.html', filename)
>> -                    labelid = node['ids'][1] # [0] is predefined by sphinx, we need [1] for explicit ones
>> +
>> +                    # node['ids'][0] contains a normalized version of the
>> +                    # headline.  If the ref and headline are the same
>> +                    # (normalized) sphinx will set the node['ids'][1] to a
>> +                    # generic id in the format `idX` where X is numeric. If the
>> +                    # ref and headline are not the same, the ref name will be
>> +                    # stored in node['ids'][1]
> 
> can you point me from where you derived that?
> 
> Because I think there are always two refs in such cases where we set one
> above a heading: the implicit heading one and the explicit from us.
> The always get normalized, but the implicit has a fallback if there's a ref
> conflict with an explicit or even another implicit one, when a title is
> reused in the same chapter or so?
> 
> Do we also have access to the chapter id/name here?
> Then we could enforce that explicit ones must have that prefixed.

I did derive that from comparing the output of the debug prints for the different situations. Unfortunately the Sphinx docs are a bit sparse on that or my search foo is not good enough ;)

Comparing the output if the explicit ref matches the implicit from the headline (shortened the 'children' element):

{'attributes': {'backrefs': [],
                 'classes': [],
                 'dupnames': [],
                 'ids': ['creating-backups', 'id1'],
                 'names': ['creating backups', 'creating_backups']},
  'children': [<title: <#text: 'Creating Backups'>>,
               <paragraph: <#text: 'This section e ...'>>,
               [.....]
               <literal_block: <#text: '# proxmox-back ...'>>,
               <section "excluding files/folders from a backup": <title...><paragraph...><paragraph...><paragraph...><par ...>],
  'document': <document: <section "backup client usage"...>>,
  'expect_referenced_by_id': {'creating-backups': <target: >},
  'expect_referenced_by_name': {'creating_backups': <target: >},


And now if the explicit ref is different from the headline:


{'attributes': {'backrefs': [],
                 'classes': [],
                 'dupnames': [],
                 'ids': ['creating-backups', 'client-creating-backups'],
                 'names': ['creating backups', 'client_creating_backups']},
  'children': [<title: <#text: 'Creating Backups'>>,
               <paragraph: <#text: 'This section e ...'>>,
               [...]
               <literal_block: <#text: '# proxmox-back ...'>>,
               <section "excluding files/folders from a backup": <title...><paragraph...><paragraph...><paragraph...><par ...>],
  'document': <document: <section "backup client usage"...>>,
  'expect_referenced_by_id': {'client-creating-backups': <target: >},
  'expect_referenced_by_name': {'client_creating_backups': <target: >},



You can see the difference in the 'attributes.ids' array.

On thing though that I observed is that 'expect_referenced_by_id' will contain the actual key used for the ref AFAICT. So we could use that and not worry about checking if the 'attributes.ids[0]' array contains a string starting with 'id[0-9]'. If I set the explicit ref to 'idX' with X being a number, that then is also present in the 'expect_referenced_by_id' field.


On an additional note: Right now we do not have any explicit references matching the headlines they are referencing because they are all prefixed or unique in another way. We could add a check here to fail if the explicit ref id matches the normalized headline and throw a warning / die with error to avoid any ambiguity in the refs in the future.

e.g. (pseudo code)
if (attributes['ids'][0] == expect_referenced_by_id:
     exit('reference is matching implicit headline ref, consider adding a prefix')

> 
>> +                    if re.match('^id[0-9]*$', node['ids'][1]):
> 
> should be a + not * op? we want to avoid clashes with real possible refs
> as much as possible..
> 
> What happens if I set now one to id1 and there would be already an id1?
> 
> I just really do not want to revisit this again, and loosing references
> is a no-go, the docs must work.

See above note, I think that addresses it.

> 
>> +                        labelid = node['ids'][0]
>> +                    else:
>> +                        labelid = node['ids'][1]
>> +
>>                       title = cast(nodes.title, node[0])
>>                       logger.info('traversing section {}'.format(title.astext()))
>>                       ref_name = getattr(title, 'rawsource', title.astext())
>>
>