Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a trivial handler for :ref: role when rendering CLI help #3440

Merged
merged 3 commits into from
Jan 27, 2025

Conversation

happz
Copy link
Collaborator

@happz happz commented Jan 2, 2025

Sphinx takes care of it when building (HTML) docs, but for CLI help, we need to provide a handler of our own. But even a simple one is enough.

Pull Request Checklist

  • implement the feature
  • extend the test coverage

@happz happz added code | no functional change "No Functional Change" intended. Patch should not change tmt's behavior in any way. code | trivial A simple patch - a couple of lines, an easy-to-understand change, a typo fix. labels Jan 2, 2025
@happz happz added this to the 1.41 milestone Jan 2, 2025
@happz happz force-pushed the rest-handle-ref-role branch from cc5f92f to ace41d1 Compare January 9, 2025 13:55
@happz happz modified the milestones: 1.41, 1.42 Jan 13, 2025
@psss psss changed the title Add a trivial handler for :ref: role when rendering CLI help Add a trivial handler for :ref: role when rendering CLI help Jan 14, 2025
@psss
Copy link
Collaborator

psss commented Jan 14, 2025

Tried to render something for the discover --how fmf plugin, but ended with this error:

TypeError: role_ref() missing 2 required positional arguments: 'options' and 'content'

@happz happz force-pushed the rest-handle-ref-role branch from ace41d1 to 79afa3e Compare January 14, 2025 14:55
@happz
Copy link
Collaborator Author

happz commented Jan 14, 2025

Tried to render something for the discover --how fmf plugin, but ended with this error:

TypeError: role_ref() missing 2 required positional arguments: 'options' and 'content'

Should be addressed in 79afa3e

@happz happz added the ci | full test Pull request is ready for the full test execution label Jan 17, 2025
@happz happz force-pushed the rest-handle-ref-role branch from 79afa3e to 2b55c06 Compare January 17, 2025 08:06
@thrix
Copy link
Collaborator

thrix commented Jan 21, 2025

Tried to render something for the discover --how fmf plugin, but ended with this error:

TypeError: role_ref() missing 2 required positional arguments: 'options' and 'content'

@psss nice catch! pls re-review

@thrix
Copy link
Collaborator

thrix commented Jan 21, 2025

The Rawhide image was updated, restart the failing test

/packit retest-failed

@thrix
Copy link
Collaborator

thrix commented Jan 21, 2025

@happz can you share which help should be improved? I am failing to find some example to verify how it looks

@happz
Copy link
Collaborator Author

happz commented Jan 21, 2025

@happz can you share which help should be improved? I am failing to find some example to verify how it looks

It's used in prepare/feature plugins documentation. And it does nothing fancy, just emits the path :ref: points to.

happz added 2 commits January 21, 2025 21:56
Sphinx takes care of it when building (HTML) docs, but for CLI help, we
need to provide a handler of our own. But even a simple one is enough.
@happz happz force-pushed the rest-handle-ref-role branch from 2b55c06 to e7f4aa2 Compare January 21, 2025 20:56
@psss
Copy link
Collaborator

psss commented Jan 22, 2025

Tried with a simple ref in the fmf plugin:

diff --git a/tmt/steps/discover/fmf.py b/tmt/steps/discover/fmf.py
index 2c4f5d6e..592af006 100644
--- a/tmt/steps/discover/fmf.py
+++ b/tmt/steps/discover/fmf.py
@@ -277,6 +277,8 @@ class DiscoverFmf(tmt.steps.discover.DiscoverPlugin[DiscoverFmfStepData]):
     when you do not have write access to the remote test repository.
     The value should follow the ``adjust`` rules syntax.
 
+    Hmmm :ref:`/spec/plans/prepare/feature`...
+
     The following example adds an ``avc`` check for each discovered
     test, doubles its duration and replaces each occurrence of the word
     ``python3.11`` in the list of required packages.

Docs are successfully built and I can see the link in the tmt run discover -h fmf --help:

  The value should follow the adjust rules syntax.

  Hmmm /spec/plans/prepare/feature...

  The following example adds an avc check for each discovered

But there are also weird warnings when building the docs:

WARNING: autodoc: failed to import module 'cli.init' from module 'tmt'; the following exception was raised:
Traceback (most recent call last):
  File "/home/psss/.local/share/hatch/env/virtual/tmt/v_yBwOy-/docs/lib/python3.13/site-packages/sphinx/ext/autodoc/importer.py", line 143, in import_module
    return importlib.import_module(modname)
           ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^
  File "/usr/lib64/python3.13/importlib/__init__.py", line 88, in import_module
  ...
  File "/home/psss/.local/share/hatch/env/virtual/tmt/v_yBwOy-/docs/lib/python3.13/site-packages/sphinx/roles.py", line 134, in create_xref_node
    'refdoc': self.env.docname,
              ^^^^^^^^
  File "/home/psss/.local/share/hatch/env/virtual/tmt/v_yBwOy-/docs/lib/python3.13/site-packages/sphinx/util/docutils.py", line 480, in env
    return self.inliner.document.settings.env
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Values' object has no attribute 'env'

@happz, are you able to reproduce the problem?

@happz
Copy link
Collaborator Author

happz commented Jan 22, 2025

Tried with a simple ref in the fmf plugin:

diff --git a/tmt/steps/discover/fmf.py b/tmt/steps/discover/fmf.py
index 2c4f5d6e..592af006 100644
--- a/tmt/steps/discover/fmf.py
+++ b/tmt/steps/discover/fmf.py
@@ -277,6 +277,8 @@ class DiscoverFmf(tmt.steps.discover.DiscoverPlugin[DiscoverFmfStepData]):
     when you do not have write access to the remote test repository.
     The value should follow the ``adjust`` rules syntax.
 
+    Hmmm :ref:`/spec/plans/prepare/feature`...
+
     The following example adds an ``avc`` check for each discovered
     test, doubles its duration and replaces each occurrence of the word
     ``python3.11`` in the list of required packages.

Docs are successfully built and I can see the link in the tmt run discover -h fmf --help:

  The value should follow the adjust rules syntax.

  Hmmm /spec/plans/prepare/feature...

  The following example adds an avc check for each discovered

But there are also weird warnings when building the docs:

WARNING: autodoc: failed to import module 'cli.init' from module 'tmt'; the following exception was raised:
Traceback (most recent call last):
  File "/home/psss/.local/share/hatch/env/virtual/tmt/v_yBwOy-/docs/lib/python3.13/site-packages/sphinx/ext/autodoc/importer.py", line 143, in import_module
    return importlib.import_module(modname)
           ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^
  File "/usr/lib64/python3.13/importlib/__init__.py", line 88, in import_module
  ...
  File "/home/psss/.local/share/hatch/env/virtual/tmt/v_yBwOy-/docs/lib/python3.13/site-packages/sphinx/roles.py", line 134, in create_xref_node
    'refdoc': self.env.docname,
              ^^^^^^^^
  File "/home/psss/.local/share/hatch/env/virtual/tmt/v_yBwOy-/docs/lib/python3.13/site-packages/sphinx/util/docutils.py", line 480, in env
    return self.inliner.document.settings.env
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Values' object has no attribute 'env'

@happz, are you able to reproduce the problem?

Hmmm, unfortunately, I am able to reproduce. It looks like some conflicts between how Sphinx sets up the ReST parser, and how we do it. Seems to me Sphinx's role handler is called instead of ours.

@happz
Copy link
Collaborator Author

happz commented Jan 24, 2025

Tried with a simple ref in the fmf plugin:

diff --git a/tmt/steps/discover/fmf.py b/tmt/steps/discover/fmf.py
index 2c4f5d6e..592af006 100644
--- a/tmt/steps/discover/fmf.py
+++ b/tmt/steps/discover/fmf.py
@@ -277,6 +277,8 @@ class DiscoverFmf(tmt.steps.discover.DiscoverPlugin[DiscoverFmfStepData]):
     when you do not have write access to the remote test repository.
     The value should follow the ``adjust`` rules syntax.
 
+    Hmmm :ref:`/spec/plans/prepare/feature`...
+
     The following example adds an ``avc`` check for each discovered
     test, doubles its duration and replaces each occurrence of the word
     ``python3.11`` in the list of required packages.

Docs are successfully built and I can see the link in the tmt run discover -h fmf --help:

  The value should follow the adjust rules syntax.

  Hmmm /spec/plans/prepare/feature...

  The following example adds an avc check for each discovered

But there are also weird warnings when building the docs:

WARNING: autodoc: failed to import module 'cli.init' from module 'tmt'; the following exception was raised:
Traceback (most recent call last):
  File "/home/psss/.local/share/hatch/env/virtual/tmt/v_yBwOy-/docs/lib/python3.13/site-packages/sphinx/ext/autodoc/importer.py", line 143, in import_module
    return importlib.import_module(modname)
           ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^
  File "/usr/lib64/python3.13/importlib/__init__.py", line 88, in import_module
  ...
  File "/home/psss/.local/share/hatch/env/virtual/tmt/v_yBwOy-/docs/lib/python3.13/site-packages/sphinx/roles.py", line 134, in create_xref_node
    'refdoc': self.env.docname,
              ^^^^^^^^
  File "/home/psss/.local/share/hatch/env/virtual/tmt/v_yBwOy-/docs/lib/python3.13/site-packages/sphinx/util/docutils.py", line 480, in env
    return self.inliner.document.settings.env
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Values' object has no attribute 'env'

@happz, are you able to reproduce the problem?

Hmmm, unfortunately, I am able to reproduce. It looks like some conflicts between how Sphinx sets up the ReST parser, and how we do it. Seems to me Sphinx's role handler is called instead of ours.

Hopefully addressed in 478fcb4. I was unable to teach Sphinx some reason, both Sphinx and docutils rely on global, module-level variables I was unable to reasonably modify just for our code. So, skipping ReST rendering of docstrings CLI would emit while running under the control of Sphinx.

@thrix thrix self-assigned this Jan 27, 2025
@thrix
Copy link
Collaborator

thrix commented Jan 27, 2025

All test failures are expected, see #3487 and rawhide is a known thing 1.41.1 in TF will fix

@thrix thrix merged commit 3427502 into main Jan 27, 2025
15 of 20 checks passed
@thrix thrix deleted the rest-handle-ref-role branch January 27, 2025 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci | full test Pull request is ready for the full test execution code | no functional change "No Functional Change" intended. Patch should not change tmt's behavior in any way. code | trivial A simple patch - a couple of lines, an easy-to-understand change, a typo fix.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants