a possible replacement for #1427

[oxarbitrage]

#1427

Added conflict detection and plugin enable code from b5362ce as i think is cleaner than what we have and I agree with #1427 (comment)

[pmconrad]

Looks good, thanks!

[cogutvalera]

Nice for me too ! Thanks ! Didn't see this PR earlier, missed it :)

[nathanielhourt]

This effectively circumvents the turbulence I mentioned in #1427. Personally, I find this approach annoying though, as this makes the software trickier and more opaque to third party developers.

with our current architecture, [...] there are no mandatory plugins. All mandatory stuff is inside the core.

Exactly! Meaning the application class, part of a core library, should not contain a magic list of plugins that automatically get loaded by default (something which would only be appropriate for a mandatory plugin). That's super weird and unexpected to third parties, who have a different perspective; one from which those plugins are useless bloat, and might not even be compiled in.

Consider a third party dev who's writing an app using Bitshares as its blockchain backend. He builds a node with a plugin that tracks the BTS blockchain for transactions generated by his app, and that node is based on the application class from the Bitshares core library, libgraphene_app. Now, he doesn't care about the markets, so he doesn't link the libgraphene_market_history library into his binary. He builds and runs, and the application class crashes on startup as it tries to load, the market_history plugin because it's on this magic list. His only option is to add a line, plugins = [] or the like, to his config.ini so this magic list gets skipped... but he can't possibly ship that to users because they don't know to do that! And he can't modify his own code to prevent this magic list from being loaded, because the list is built into a core bitshares library. His only option is to fork and create a version of

If there's a standard list of plugins that the bitshares developers expect to be loaded, that list should live in the bitshares developers' playground (in the programs directory, i.e. programs/witness_node/main.cpp, or in the tests directory... Not in the libraries directory) and not in the core libraries, which need to be generic so third party applications can link against them.

[cogutvalera]

@nathanhourt to integrate your solution we need to break existing installations, this PR just for enhancing old code, but your PR is to make changes which makes sense for me because I absolutely agree that libraries must be generic without such magic list. We need to discuss it in your PR IMHO, and not in this PR.

Thanks !

[oxarbitrage]

Your point is valid to me @nathanhourt . We are all interested in development using bitshares as a backend.
I think another possible alternative will be:

• Use Nathan code as it is in: Improve plugin loader #1427
• Then add default plugin list to https://github.com/bitshares/bitshares-core/blob/develop/programs/witness_node/main.cpp (we are going to need duplicated if (options.count("plugins")) { code).
• This is done by using the new autoload flag to at registration time: https://github.com/bitshares/bitshares-core/blob/develop/programs/witness_node/main.cpp#L73-L81
• Check test cases are working.

[oxarbitrage]

If we do this, we should also consider api_access defaults and seed nodes in the future.

[oxarbitrage]

This is a bit related: #533

@nathanhourt when you don't link the libgraphene_market_history you can still compile witness_node ?
The optional build of plugins is something i want to see/do at some time but it is too much code change, headers for plugins are everywhere. I think it will be something(decrease binary overall size) if we build all plugins as we do now but at least don't link them to the main binary if that is even possible.

Any comment in that issue will be appreciated too.

[nathanielhourt]

@oxarbitrage Truth be told, that part of my narrative is a bit of a stretch, as I haven't actually tried to remove linking of the plugins I don't use... But I do skip the register_plugin() calls at runtime for the plugins I don't care about, which also causes application to fail out on the magic list.

[pmconrad]

Meaning the application class, part of a core library, should not contain a magic list of plugins that automatically get loaded by default

I agree.
Thinking further, this means that the --plugins setting should not be owned by the application library but by the executable (i. e. witness_node or delayed_node). Note that delayed_node already uses a different default.

[oxarbitrage]

closing this one as the best candidate is #1437

will be merged after getting 1 approval.