sphinx-needs icon indicating copy to clipboard operation
sphinx-needs copied to clipboard

Processing warnings before post-process of needs builder

Open malsabbagh-mhp opened this issue 1 year ago • 6 comments

I've just identified the following bug/issue in the needs builder:

  • We configure the values of a needs option using a dynamic function (:custom_option: [[get_the_value()]]).
  • It's worth noting that the custom_option value can also be manually specified.
  • Consequently, we verify the custom_option value through a registered warnings function.
  • Upon inspecting Sphinx-Needs 2.0.0, I observed the addition of a post-processing step, allowing, for instance, the invocation of dynamic functions.
  • However, an issue arises when using dynamic functions because the validation or invocation of warnings occurs before post-processing. As a result, the value [[get_the_value()]] gets passed into the warning function.

My suggestion would be to move the following line: app.connect("build-finished", process_warnings) after processing build_needumls_pumls or debug.process_timing

https://github.com/useblocks/sphinx-needs/blob/master/sphinx_needs/needs.py#L245-L249`

@danwos As discussed here the issue

malsabbagh-mhp avatar Feb 02 '24 07:02 malsabbagh-mhp

the validation or invocation of warnings occurs before post-processing.

Heya @malsabbagh-mhp, perhaps you can clarify with more information, or a minimal reprocuble example, but I do not believe this to be the case:

I believe you are refering to the post-processing here: https://github.com/useblocks/sphinx-needs/blob/73b961e29583bc0ac8892ef9c4e2bfb75f7f46bb/sphinx_needs/builder.py#L55-L56

This is called at the end of the build step, before the build-finished is emitted: https://github.com/sphinx-doc/sphinx/blob/5e9550c78e3421dd7dcab037021d996841178f67/sphinx/application.py#L339-L352

you can see this, if you add print statements at the start of process_warnings and NeedsBuilder.finish, then run the needs builder:

...
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
***** post-processing needs data
Needs successfully exported
***** process_warnings

Checking sphinx-needs warnings
...

chrisjsewell avatar Feb 08 '24 18:02 chrisjsewell

Hi @chrisjsewell,

Thank you for your response. I've been working with a custom builder that extends the Builder class and overrides the following functions: write, write_doc, get_target_uri, finish, get_outdated_docs, prepare_writing, and write_doc_serialized.

When I execute the sphinx-build command with my custom builder, I encounter the following logs:

*****
checking consistency... done
dumping object inventory... done

Checking sphinx-needs warnings
*****
Needs successfully exported
build succeeded.

From what I gather:

  • When using the needs builder, the finish function is executed before processing warnings, which aligns with expectations.
  • However, when utilizing a custom builder, the registered build_needs_json function is invoked after process_warning, resulting in the value [[get_the_value()]] being passed into the warning function without prior processing.

malsabbagh-mhp avatar Feb 09 '24 09:02 malsabbagh-mhp

@malsabbagh-mhp ah if you are writing your own builder (which bypasses the sphinx write phase like the NeedsBuilder) then you also need to specifically call post_process_needs_data(self.app)

Note, as it says in the function docstring, the function is idempotent, so you can call it as many times as you want and it will only process the needs once.

We have previously discussed, that ideallypost_process_needs_data would be added as a Sphinx event, that always runs in-between the read and write phase, and then you would not have to add it manually. However, currently this event does not exist.

chrisjsewell avatar Feb 09 '24 13:02 chrisjsewell

@chrisjsewell thanks for your reply.

In my conf.py, I've configured needs_build_json to True, and Sphinx-needs automatically sets up the call to build_needs_json upon completion of the build process. You can refer to this link.

Given this setup, my expectation is that Sphinx-needs would trigger build_needs_json to generate the needs.json file before processing any warnings.

Therefore I am not sure, why should I call post_process_needs_data(self.app)? Could you please explain it, why are you generating the needs.json after processing the warnings?

Many thanks for your time! Br, Mohamed.

malsabbagh-mhp avatar Feb 09 '24 16:02 malsabbagh-mhp

any update on this?

mpaxson avatar Feb 20 '24 13:02 mpaxson

Therefore I am not sure, why should I call post_process_needs_data(self.app)?

Because if your custom builder is intending to use the needs data in anyway, if post_process_needs_data(self.app) is not called before you start the "write phase" then the needs data will be wrong (because it will not be post-processed)

Could you please explain it, why are you generating the needs.json after processing the warnings?

It really doesn't which way round they are: the post-processing of needs is intended to run before both of them; build_needs_json is intended simply to write the existing data to disk

chrisjsewell avatar Feb 22 '24 17:02 chrisjsewell

Solved by adding post_process_needs_data(self.app) to the finish(self) function. Thanks!

malsabbagh-mhp avatar Mar 18 '24 09:03 malsabbagh-mhp