On 04/02/2017 09:40 PM, Akash Rawal wrote: > > > On 04/02/2017 10:41 PM, Darshit Shah wrote: >> * Akash Rawal <[email protected]> [170331 23:34]: >>> Hello, >>> >>> First of all, thanks for the initial review of my proposal! >>> >>> I have made some changes in response to the comments as well as last >>> mail on the mailing list by Darshit Shah. Kindly consider reviewing it. >>> For reference here is the link. >>> https://docs.google.com/document/d/1SBM-jNG5dJL85C7iK9aid2eDlByrxOsYUNDrk4049vQ/edit?usp=sharing >>> >>> >>> >>> The proposal is still not final, in the sense that I am analysing the >>> code for further ideas of what can be customized using plugins. >>> >>> I was asked to elaborate on rewriting of command line option processing >>> code. So here are my thoughts on it. >>> >>> The current code in src/options.c has following drawbacks: >>> - Built around order-insensitive and idempotent model (Supplying >>> an option modifies a variable). This might be adequete for wget, >>> but might not be the case for plugins. Repeated options or >>> order of options can matter for plugins. >> Breaking the order-insensitive and idempotent model of command line >> processing is an absolute no-go. It will definitely break many scripts >> out there. We do wish to make it easy to transition from Wget to Wget2 >> and this is a huge change in how options are processed. Also, I see no >> clear advantages to why this should be required. > Backward compatibility will only be broken if options are changed, > which I didn't plan to do. wget2 will support all options it > supports now. Existing options shall remain order-insensitive > and idempotent, only new options and plugin-side options may > take advantage of greater flexibility. > > Many programs have order-sensitive (e.g. ffmpeg, options > apply to input or output file based on placement) and non-idempotent > (e.g. -v (verbose) option in many program, -vv for higher verbosity) >>> - The need for keeping the list of options sorted. I don't think >>> we can expect plugin writers to do the same. >> No. Plugins should not be allowed to tinker with certain core >> functionalities and that includes how options are processed. >>> - And most importantly, options cannot be added at runtime. >> Which is a very good thing. Keeping everything at compile time means we >> can have most of that code on the stack without the need for costly >> dynamic memory allocations on each invokation. Options processing code >> should definitely not be dynamic. > Okay if that is your requirement I'll remove it from my proposal. >>> >>> So far I think rewriting the option processing code would be easier >>> than trying to patch existing code to support custom options. >>> Some parts of the original code could be reused but structure >>> would undergo a drastic change anyway. >> >> Easier? Have you every tried writing one? The number of use cases and >> corner cases you'd have to handle is massive. That alone could be an >> entire GSoC project. You can't expect to write it as a byproduct of your >> project. > I have written simpler ones. But even in wget's code option processor is > not so large (~400 LOC, starting from src/option.c:853) > Cases for long options: > --option > --option=value > --option value > --no-option > Cases for short options: > -x > -x value > -xvalue > -xyzw > -xyzvalue > -xyz value >> >>> >>> The idea I have is to expose an interface for plugins to describe >>> command line options they want to receive. I have attached a >>> prototype header. From this interface, wget will have a list of >>> command line options to support, along with plugin function to >>> be called for handling plugin specific options. In addition to >>> using this to parse command line, wget can Use the descriptions >>> and category information provided to generate --help output. >> >> First let me ask you, how exactly do you envision a user using plugins >> with Wget2? How are plugins selected? How does the user even know if a >> plugin exists? > How about: > $ wget_plugins="foo" wget2 --foo-option http://example.com/file.html > Or: > $ wget2 --plugin=foo --foo-option http://example.com/file.html > Hypothetical example: music site parser that filters by rating > $ wget2 --plugin=music_rating_filter --music_min_rating=4 \ > --recursive --level=20 \ > http://example_music_site.com/music/1 > Multiple plugins: > $ wget_plugins="foo:bar" wget2 --plugin=foo --plugin=bar \ > --foo-option --bar-option=value http://example.com/file.html > Or: > $ wget2 --plugin=foo --plugin=bar --foo-option --bar-option=value \ > http://example.com/file.html > $ #The above is also an example of non-idempotent option --plugin > Loading plugins which are not installed: > $ wget2 --local-plugin=/path/to/libfoo.so --foo-option \ > http://example.com/file.html > > And regarding user knowing which plugins are installed: > $ wget2 --list-plugins > foo > bar > music_rating_filter > Plugins could be installed in a directory like > ${PREFIX}/lib/wget/plugins/. Many software do like this. >>> >>> Regarding the need to support plugin side addition of command line >>> options at the first place, here are my thoughts. >>> Plugins might need additional data from the user. I can think >>> of 3 ways in which it can be done: >>> 1. Environment variables. >>> 2. Files with well-known names. >>> 3. Command line. >>> Environment variables have a disadvantage that they are automatically >>> passed to the subprocesses, creating a chance for interference. >>> Also, name clashes cannot be discovered. >>> >>> Using files with well-known names has a chance of interference >>> with parallel running instances of wget. >>> >>> None of these disadvantages are shared with using command line >>> options. >> >> However, all three are commonly used to provide options to a program and >> should all always be supported. Environment variables allow for all >> instances in one subshell to share a set of commands without having to >> pass them multiple times and without interfering with instances in other >> subshells >> Configuration files are useful so the system administrator can set a >> bunch of common options for all users, maybe to enforce a policy. >> Command line options give the user the power to modify options on a per >> invokation level. > Per invocation level is actually my primary concern. >> >>> >>> Opinions are welcome. >>> >>> Regards, >>> Akash Rawal. >>> >>> >>> > > So the summary is, due to the requirement of running the code on > stack as much as possible, I'm removing the customizable option > processing from my proposal.
Sorry when I drop in without having read/understood all... > Multiple plugins: > $ wget_plugins="foo:bar" wget2 --plugin=foo --plugin=bar \ > --foo-option --bar-option=value http://example.com/file.html I like this. We can have this without changing much of Wget2's option processing code. Add --plugin to Wget2 (we will have a list/array of plugin names than). Define a option name scheme to identify options for a certain plugin, e.g. --plugin-foo-<option-name>=value. Make as as simple as possible, e.g. a value is mandatory (bool is 1|0 or y|n or whatever). When the current option processing doesn't find an option in it's list, we special check for --plugin-<plugin-name>-<option-name> and give the key/value pair <option-name>/<value> to a function that is provided by the plugin (e.g. wget_plugin_set_option(const char *option_name, const char *value). With this you can provide CLI options to plugins without changing much in Wget2's code. Regards, Tim
signature.asc
Description: OpenPGP digital signature
