harald commented 2025年08月18日 20:32:25 +02:00

Current Situation

The targets are stored in the global variable _TARGETS and _STATEFILE is also global.

This is OK for all normal use cases. Running several builds in the same process with differing sets of targets and separate state storage is not a supported use case.

Yet during testing completely independent builds are initiated, all run in the same test process with a good chance to mess up the state for each other and fun with hard to debug nonsense.

Requirement

Provide a common global state re-initialization to be run by all tests.

To find all global mutual state, check pydoctor output with "show private api".

In the process annotate constants as Final and potentially rename non-constants to not be all-caps.

Consider the following idea, even if we actually want this only for testing purposes. It should get rid of global variables spilled all over the place.

Ideas

Create a class Build which keeps the state of a build, in particular

• the list of targets,
• the state file or some other sink to store the state hashes,
• the log level

In order to not be forced to explicitly add a target to a Build or pass a Build to the Target constructor, a compromise may be to have one global variable, currentBuild which is used in all places.

To make using different Build easy, we might have a stack and work like

newBuild() # pushes a fresh build on a stack
...
finishBuild() # pops the top stack

Initially, a newBuild() is called automagically.

# Current Situation The targets are stored in the global variable `_TARGETS` and `_STATEFILE` is also global. This is OK for all normal use cases. Running several builds in the same process with differing sets of targets and separate state storage is not a supported use case. Yet during testing completely independent builds are initiated, all run in the same test process with a good chance to mess up the state for each other and fun with hard to debug nonsense. # Requirement Provide a common global state re-initialization to be run by all tests. To find all global mutual state, check pydoctor output with "show private api". In the process annotate constants as `Final` and potentially rename non-constants to not be all-caps. Consider the following idea, even if we actually want this only for testing purposes. It should get rid of global variables spilled all over the place. # Ideas Create a class `Build` which keeps the state of a build, in particular - the list of targets, - the state file or some other sink to store the state hashes, - the log level In order to not be forced to explicitly add a target to a `Build` or pass a `Build` to the `Target` constructor, a compromise may be to have one global variable, `currentBuild` which is used in all places. To make using different `Build` easy, we might have a stack and work like ```python newBuild() # pushes a fresh build on a stack ... finishBuild() # pops the top stack ``` Initially, a `newBuild()` is called automagically.

harald commented 2025年08月29日 11:41:26 +02:00

Fixed with 741e40d738.

And it was only fixed for tests. Rationale:

• The idea with newBuild() and currentBuild() to somehow hide the Build object from normal use cases introduced the very ugly use of currentBuild() calls all over the place in pythonbuilder's code.
• The "correct" implementation then would be to officially force even normal use cases to use a Build object, maybe with something like

  build = Build()
  build.target(...) # same parameters as Target()
  

  but all this for a non-supported use case of having more than one build in the same process?

Fixed with 741e40d738. And it was only fixed for tests. Rationale: - The idea with newBuild() and currentBuild() to somehow hide the Build object from normal use cases introduced the very ugly use of currentBuild() calls all over the place in pythonbuilder's code. - The "correct" implementation then would be to officially force even normal use cases to use a Build object, maybe with something like ```python build = Build() build.target(...) # same parameters as Target() ``` but all this for a non-supported use case of having more than one build in the same process?