Lack of interest More Control Over Development Output

[pegasus]

While writing Setup classes for add-ons, I have been fighting with XF:DevelopmentOutput. Whenever I need to write Entities with XF:DevOutputWritable Behavior, I need to explicitly interact with each Entity, get the Behavior, and set the option write_dev_output to false before saving; I cannot use XenForo's built-in repository code for making quick updates to things like Options for example (like if an option's format has changed between versions, and we need to migrate old values).

Why? The XF:DevOutputWritable behavior will create a new _output folder with only the touched add-on data in it, and the final XML import steps will read from that sparse _output folder rather than from _data. So basically no templates, routes, or anything get imported.

In some situations even Entities (such as XF:BbCode) invoke relations such as XF:phrase, where it is not possible to change the Behavior at all (class-extensions not being imported yet/or disabled by the add-on's is_processing flag).

I propose a number of better solutions to this:

• Allow direct interaction with XF:DevelopmentOutput. Add methods like ::enable(), ::disable(), ::addSkippedAddOn. This way we can just disable it in the Setup steps if we need to, and don't have to recode things that are already in core. It may be prudent to just make the add-on installer/upgrader automatically ::addSkippedAddOn the add-on that is being installed/upgraded, due to the above reasons.
• Ensure that relations using XF:DevOutputWritable, if they are invoked by an entity also with that Behavior, inherit the write_dev_output value of the parent that invoked them.

Side note: Sometimes it can be useful to use our class extensions and listeners within our upgrade steps (it could have been easier to deal with this issue, for one). I propose that is_processing does not affect upgrade steps, or that there is an easier way to load into memory, on-the-fly from within Setup, the things which are omitted due to the is_processing flag. Currently I work around the issue by turning off is_processing and rebuilding as the first step of all upgrades. This is not ideal because it reopens the original issues is_processing is intended to prevent (user-facing errors while the upgrade is_processing), if your add-on does not implement its own is_processing option and check it in every listener, extension, and handler.

 

[pegasus]

I should follow up by saying that XF\Entity\StyleProperty and most entities I checked correctly pass the option to XF:DevOutputWritable on XF\Entity\Phrase, but XF\Entity\BbCode does not and needs to be fixed. It would be worth checking all entities that manage phrases (or other add-on related data) to ensure this.

 

[Mike]

Overall, I'm a bit confused by what you're actually after with this suggestion, but there's one comment that makes me wonder if there's an underlying assumption that isn't correct:

Why? The XF:DevOutputWritable behavior will create a new _output folder with only the touched add-on data in it, and the final XML import steps will read from that sparse _output folder rather than from _data. So basically no templates, routes, or anything get imported.

I added the bold. Why do you have a sparse _output folder? The development output should really either fully be there or not there are at all. If you want the output to be created from what's in the DB, you just have to run the xf-dev:export command. If you don't want dev output for an add-on at all (though we wouldn't recommend this), you should opt-out via config.php.

Much of the rest of your post seems to be based around trying to enable/disable development output in specific scenarios, though I don't see why this is generally necessary if you don't have the sparse output data in the first place.

This element also relates in a different way:

Side note: Sometimes it can be useful to use our class extensions and listeners within our upgrade steps (it could have been easier to deal with this issue, for one).

In a similar way to core upgrades, your upgrade (and install/uninstall steps) should strive to be as self contained as possible. In terms of an add-on, this really means not depending on anything outside of the XF core. (For XF itself, it generally means avoiding the entirety of the XF core.) Otherwise, you will potentially create situations where older upgrade steps break when you make changes in later versions. As a somewhat extreme example, while the code has been updated the upgrade steps for XFRM 1.x were ported over basically directly into XFRM 2.0 because they work at a low level.

On a side note, if you really do need your listeners and the like, you'd want to use the postInstall/postUpgrade Setup methods to trigger jobs. Though note that this would be after the data is imported.

 

[pegasus]

Let's say @Mike is installing this add-on, on a different forum than it was developed on, because @pegasus developed it on his own forum. @Mike will only have the _data folder, not the _output folder.

In the problematic scenario, Mike has dev-mode active on his forum, so as designed, his forum will attempt XF:DevOutputWritable in certain entities. Regarding your suggestion, we should not rely on Mike to use xf-dev:export on pegasus's add-on if Mike has not installed it yet. That seems insane to me, puts an unnecessary burden on Mike, and I don't think it would work.

Now, let's say pegasus's add-on's Setup script contains calls to core XF entities like XF:BbCode.

1. Mike attempts to install the add-on.
2. Setup calls ::save on those entities in its custom steps. We do not want to trigger XF:DevOutputWritable, so those steps specifically pass the write_dev_output option (false) in the entities.
3. XF:DevOutputWritable happens anyway, because Mike has dev-mode turned on and because of some bugs in XF core.
4. XF:DevOutputWritable generates a brand new _output folder for the add-on, containing the ::save[d] entries. Now there is an unwanted _output folder that contains fewer entries than _data contains.
5. XF's add-on installer attempts to import the add-on XML. dev-mode is on, and it sees an _output folder, so it uses that instead of _data.
6. The installer says the installation is complete (but you and I know it has failed).

As mentioned in my second post, specifically, there is a bug in the core XF:BbCode entity where the internal XF:phrase entities do not inherit the XF:DevOutputWritable value. This bug is what led to my problem, and thereafter my suggestion. However, even if the bug is fixed, I still believe that added protection against this scenario should be employed (e.g. do not permit generating an _output folder for an add-on, if the same process is running Setup; that is, when entering Setup, disable XF\DevelopmentOutput for this process).

 

[pegasus]

This bug still exists in 2.1.0 RC 2.

there is a bug in the core XF:BbCode entity where the internal XF:phrase entities do not inherit the XF:DevOutputWritable value.

The bug was the underlying issue that caused me to create this thread.

 

[Mike]

I have now adjusted that particular instance.